NE-2723: Migrate to aws-sdk-go-v2 by rikatz · Pull Request #1445 · openshift/cluster-ingress-operator

rikatz · 2026-05-13T21:36:41Z

What

Migrate the AWS DNS provider from the deprecated aws-sdk-go (v1) to aws-sdk-go-v2.

Why

github.com/aws/aws-sdk-go reached end-of-support on July 31, 2025 and no longer receives security patches, bug fixes, or new service/region support. The cluster-ingress-operator uses the AWS SDK for Route53 DNS management, ELB/ELBv2 load balancer discovery, tag-based resource lookups, and STS cross-account role assumption — all of which need to stay on an actively maintained SDK.

Changes

Core DNS provider (pkg/dns/aws/dns.go): Replaced session.NewSession with config.LoadDefaultConfig, migrated all four service clients (Route53, ELB, ELBv2, Resource Groups Tagging API) to NewFromConfig, converted callback-based pagination (*Pages) to iterator-based paginators (NewXxxPaginator), switched error handling from awserr.Error type assertions to smithy.APIError with errors.As, and migrated all pointer-type slices ([]*string, []*route53.Tag) to value-type slices ([]string, []r53types.Tag)
Endpoint resolution: Replaced the hand-rolled resolveEndpoint function with per-service calls to the SDK's own NewDefaultEndpointResolverV2(), ensuring endpoint URLs for logging and test assertions match the SDK's actual per-request resolution rather than a manually maintained lookup table
Removed hardcoded endpoint constants: Eliminated chinaRoute53Endpoint, govCloudRoute53Region, and govCloudTaggingEndpoint — the SDK v2 natively resolves the correct endpoints for all partitions (aws, aws-cn aws-us-gov, aws-iso, aws-iso-b, aws-eusc)
GovCloud detection: Replaced endpoint URL string inspection (strings.Contains(endpoint, "us-gov")) with region-based check (isGovCloudRegion), which is equivalent since cross-partition access is not supported by AWS
STS role assumption: Replaced stscreds.NewCredentials(sess, roleARN) with stscreds.NewAssumeRoleProvider(stsClient, roleARN) wrapped in aws.NewCredentialsCache
User-agent middleware: Replaced v1 request.WithAppendUserAgent handler with v2 awsmiddleware.AddUserAgentKeyValue
Service name constants: Hardcoded "route53", "elasticloadbalancing", "tagging" since v2 only exports human-readable ServiceIDs (e.g. "Route 53") which don't match the OpenShift AWSServiceEndpoint.Name API
Test coverage: Added tests for partitionIDForRegion (10 cases), isGovCloudRegion (6 cases), GovCloud West default endpoints, GovCloud East/West custom endpoint behavior, and EUSC Brandenburg default endpoints (45 total, up from 32)
Defensive fix: Added guard against calling ListTagsForResources with an empty ResourceIds list when a paginator page returns zero hosted zones
E2E test files: Migrated aws_util_test.go, lb_eip_test.go, and operator_test.go to v2 types, replaced deprecated ioutil calls with io/os equivalents, removed deprecated rand.Seed
Dependencies: Replaced github.com/aws/aws-sdk-go with 10 aws-sdk-go-v2 modules and github.com/aws/smithy-go

openshift-ci-robot · 2026-05-13T21:36:46Z

@rikatz: This pull request references NE-2723 which is a valid jira issue.

Details

In response to this:

The aws-sdk-go library, used on DNS controller is deprecated and does not receive updates anymore.

This change migrates to the new aws-sdk-go-v2, adding the new library and making the required changes to keep the code compatible

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

coderabbitai · 2026-05-13T21:36:55Z

📝 Walkthrough

Walkthrough

The PR migrates dependencies to AWS SDK for Go v2 and updates code to use v2 clients and types: rewrites the AWS Route53 DNS provider (partition-aware endpoints, tagging discovery, ELB discovery, record changes), updates shared-credentials helpers and tests to v2 config/temp-file semantics, and converts unit and e2e tests/utilities from SDK v1 to v2 while replacing deprecated ioutil usages.

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Ote Binary Stdout Contract	❌ Error	PR introduces fmt.Printf() calls in TestMain() function in test/e2e/operator_test.go, which writes to stdout and violates OTE Binary Stdout Contract that requires JSON-only stdout.	Replace fmt.Printf() with logging to stderr, or use t.Log/t.Logf, or redirect stderr via os.Stderr in TestMain error paths.
Docstring Coverage	⚠️ Warning	Docstring coverage is 59.38% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Test Structure And Quality	❓ Inconclusive	Custom check specifies "Ginkgo test code" but repository uses standard Go testing (Test* functions). No Ginkgo imports exist in go.mod; 0 Describe blocks found, 86 Test* functions found.	Clarify if check applies to standard Go tests. Standard Go test analysis: Most assertions lack messages; single-responsibility generally followed; cleanup uses t.Cleanup or defer; timeouts present in e2e tests.

✅ Passed checks (9 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: migrating the codebase from AWS SDK v1 to v2. It is specific, relevant, and matches the changeset across all affected files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	No Ginkgo framework used in PR. All test names are stable static strings with no dynamic values (timestamps, UUIDs, random identifiers). Test case names are descriptive or region-based identifiers.
Microshift Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added in this PR. All test modifications use standard Go testing.T framework, not Ginkgo.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added. The PR adds only standard Go unit tests (Test_partitionIDForRegion, Test_isGovCloudRegion) with no cluster topology dependencies.
Topology-Aware Scheduling Compatibility	✅ Passed	PR migrates AWS SDK v1→v2 in DNS controller and tests; no deployment manifests, pod specs, or scheduling constraints were added/modified.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added. Modified e2e test files are helper functions only. New unit tests (Test_partitionIDForRegion, Test_isGovCloudRegion) lack IPv4 or external connectivity issues.
Description check	✅ Passed	The pull request description comprehensively details the migration from aws-sdk-go v1 to v2, explaining why (end-of-support), what changed (client migration, endpoint resolution, error handling), and specific implementation details across multiple files and test suites.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

test/e2e/lb_eip_test.go (1)

443-505: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Return false when EIP release fails or unassociated EIPs still exist

Line 495 can return success even when releases failed (Line 489) or when unassociated EIPs were only attempted to be released. That can stop polling early and leak EIPs.

💡 Suggested fix

 func cleanupEIPAllocations(t *testing.T, svc *ec2.Client, clusterName string) bool {
@@
-	// Release each unassociated EIP
+	// Release each unassociated EIP
+	releaseFailed := false
 	for _, eip := range unassociatedEIPs {
 		_, err := svc.ReleaseAddress(context.TODO(), &ec2.ReleaseAddressInput{
 			AllocationId: eip.AllocationId,
 		})
 		if err != nil {
 			t.Errorf("Failed to release EIP %v with Allocation ID %v: %v", aws.ToString(eip.PublicIp), aws.ToString(eip.AllocationId), err)
+			releaseFailed = true
 		} else {
 			t.Logf("Released EIP %v with Allocation ID %v", aws.ToString(eip.PublicIp), aws.ToString(eip.AllocationId))
 		}
 	}
 
-	if len(associatedEIPs) == 0 {
+	// Keep polling until previously-unassociated EIPs are gone from a fresh DescribeAddresses call.
+	if releaseFailed || len(unassociatedEIPs) > 0 {
+		return false
+	}
+	if len(associatedEIPs) == 0 {
 		t.Log("No associated EIPs found with the specified tag key and value.")
 		return true
 	} else {

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/e2e/lb_eip_test.go` around lines 443 - 505, cleanupEIPAllocations
currently can return true even when releasing unassociated EIPs failed or when
unassociated EIPs were only attempted to be released; update the function to
track release failures and only return true when there were no release errors
and there are no remaining unassociated EIPs (and associatedEIPs is empty).
Concretely: inside cleanupEIPAllocations, add a boolean (e.g., releaseFailed)
set to true whenever svc.ReleaseAddress returns an error (instead of just
t.Errorf), and after the release loop treat any releaseFailed or any non-empty
unassociatedEIPs as a failure path (return false); only return true when
associatedEIPs is empty AND releaseFailed is false. Reference:
cleanupEIPAllocations, unassociatedEIPs, associatedEIPs, svc.ReleaseAddress.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@test/e2e/lb_eip_test.go`:
- Around line 443-505: cleanupEIPAllocations currently can return true even when
releasing unassociated EIPs failed or when unassociated EIPs were only attempted
to be released; update the function to track release failures and only return
true when there were no release errors and there are no remaining unassociated
EIPs (and associatedEIPs is empty). Concretely: inside cleanupEIPAllocations,
add a boolean (e.g., releaseFailed) set to true whenever svc.ReleaseAddress
returns an error (instead of just t.Errorf), and after the release loop treat
any releaseFailed or any non-empty unassociatedEIPs as a failure path (return
false); only return true when associatedEIPs is empty AND releaseFailed is
false. Reference: cleanupEIPAllocations, unassociatedEIPs, associatedEIPs,
svc.ReleaseAddress.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 2c5b648a-c0c5-417c-ba11-1ec530d375b3

📥 Commits

Reviewing files that changed from the base of the PR and between 10bbe5f and 9470c72.

⛔ Files ignored due to path filters (292)

go.sum is excluded by !**/*.sum
vendor/github.com/aws/aws-sdk-go-v2/LICENSE.txt is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/NOTICE.txt is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/accountid_endpoint_mode.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/arn/arn.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/checksum.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/config.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/context.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/credential_cache.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/credentials.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/defaults/auto.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/defaults/configuration.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/defaults/defaults.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/defaults/doc.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/defaultsmode.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/doc.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/endpoints.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/errors.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/from_ptr.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/go_module_metadata.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/logging.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/logging_generate.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/metadata.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/middleware.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/osname.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/osname_go115.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/recursion_detection.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/request_id.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/request_id_retriever.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/user_agent.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/ec2query/error_utils.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/array.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/encoder.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/map.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/middleware.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/object.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/value.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/restjson/decoder_util.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/xml/error_utils.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/ratelimit/none.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/ratelimit/token_bucket.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/ratelimit/token_rate_limit.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/request.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/retry/adaptive.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/retry/adaptive_ratelimit.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/retry/adaptive_token_bucket.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/retry/attempt_metrics.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/retry/doc.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/retry/errors.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/retry/jitter_backoff.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/retry/metadata.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/retry/middleware.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/retry/retry.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/retry/retryable_error.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/retry/standard.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/retry/throttle_error.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/retry/timeout_error.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/retryer.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/runtime.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/cache.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/const.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/header_rules.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/headers.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/hmac.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/host.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/scope.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/time.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/util.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/middleware.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/presign_middleware.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/stream.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/v4.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/to_ptr.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/client.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/content_type.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/response_error.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/response_error_middleware.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/timeout_read_closer.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/types.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/aws/version.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/config/CHANGELOG.md is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/config/LICENSE.txt is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/config/auth_scheme_preference.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/config/config.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/config/defaultsmode.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/config/doc.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/config/env_config.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/config/generate.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/config/go_module_metadata.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/config/internal/ini/errors.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/config/internal/ini/ini.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/config/internal/ini/parse.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/config/internal/ini/sections.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/config/internal/ini/strings.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/config/internal/ini/token.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/config/internal/ini/tokenize.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/config/internal/ini/value.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/config/load_options.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/config/local.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/config/provider.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/config/resolve.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/config/resolve_bearer_token.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/config/resolve_credentials.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/config/shared_config.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/credentials/CHANGELOG.md is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/credentials/LICENSE.txt is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/credentials/doc.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/credentials/ec2rolecreds/doc.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/credentials/ec2rolecreds/provider.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/internal/client/auth.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/internal/client/client.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/internal/client/endpoints.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/internal/client/middleware.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/provider.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/credentials/go_module_metadata.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/credentials/logincreds/dpop.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/credentials/logincreds/file.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/credentials/logincreds/provider.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/credentials/logincreds/token.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/credentials/processcreds/doc.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/credentials/processcreds/provider.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/doc.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/sso_cached_token.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/sso_credentials_provider.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/sso_token_provider.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/credentials/static_provider.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/credentials/stscreds/assume_role_provider.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/credentials/stscreds/web_identity_provider.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/CHANGELOG.md is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/LICENSE.txt is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_client.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetDynamicData.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetIAMInfo.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetInstanceIdentityDocument.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetMetadata.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetRegion.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetToken.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetUserData.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/auth.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/doc.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/endpoints.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/go_module_metadata.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/internal/config/resolvers.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/request_middleware.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/token_provider.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/auth/auth.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/auth/scheme.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/bearer_token_adapter.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/bearer_token_signer_adapter.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/credentials_adapter.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/smithy.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/v4signer_adapter.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/CHANGELOG.md is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/LICENSE.txt is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/config.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/endpoints.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/go_module_metadata.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/context/context.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/arn.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/doc.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/generate.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/host.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partition.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partitions.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partitions.json is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/endpoints.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/CHANGELOG.md is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/LICENSE.txt is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/endpoints.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/go_module_metadata.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/rand/rand.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/sdk/interfaces.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/sdk/time.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/sdkio/byte.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/shareddefaults/shared_config.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/strings/strings.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/sync/singleflight/LICENSE is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/sync/singleflight/docs.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/sync/singleflight/singleflight.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/timeconv/duration.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/CHANGELOG.md is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/LICENSE.txt is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/credentials.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/error.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/go_module_metadata.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/crypto/compare.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/crypto/ecc.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/const.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/header_rules.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/headers.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/hmac.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/host.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/time.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/util.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/middleware.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/presign_middleware.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/smithy.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/v4a.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/CHANGELOG.md is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/LICENSE.txt is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_client.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptAddressTransfer.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptCapacityReservationBillingOwnership.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptReservedInstancesExchangeQuote.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptTransitGatewayClientVpnAttachment.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptTransitGatewayMulticastDomainAssociations.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptTransitGatewayPeeringAttachment.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptTransitGatewayVpcAttachment.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptVpcEndpointConnections.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptVpcPeeringConnection.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AdvertiseByoipCidr.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AllocateAddress.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AllocateHosts.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AllocateIpamPoolCidr.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_ApplySecurityGroupsToClientVpnTargetNetwork.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssignIpv6Addresses.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssignPrivateIpAddresses.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssignPrivateNatGatewayAddress.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateAddress.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateCapacityReservationBillingOwner.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateClientVpnTargetNetwork.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateDhcpOptions.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateEnclaveCertificateIamRole.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateIamInstanceProfile.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateInstanceEventWindow.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateIpamByoasn.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateIpamResourceDiscovery.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateNatGatewayAddress.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateRouteServer.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateRouteTable.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateSecurityGroupVpc.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateSubnetCidrBlock.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateTransitGatewayMulticastDomain.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateTransitGatewayPolicyTable.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateTransitGatewayRouteTable.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateTrunkInterface.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateVpcCidrBlock.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AttachClassicLinkVpc.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AttachInternetGateway.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AttachNetworkInterface.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AttachVerifiedAccessTrustProvider.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AttachVolume.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AttachVpnGateway.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AuthorizeClientVpnIngress.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AuthorizeSecurityGroupEgress.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AuthorizeSecurityGroupIngress.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_BundleInstance.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CancelBundleTask.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CancelCapacityReservation.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CancelCapacityReservationFleets.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CancelConversionTask.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CancelDeclarativePoliciesReport.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CancelExportTask.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CancelImageLaunchPermission.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CancelImportTask.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CancelReservedInstancesListing.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CancelSpotFleetRequests.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CancelSpotInstanceRequests.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_ConfirmProductInstance.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CopyFpgaImage.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CopyImage.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CopySnapshot.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CopyVolumes.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateCapacityManagerDataExport.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateCapacityReservation.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateCapacityReservationBySplitting.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateCapacityReservationFleet.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateCarrierGateway.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateClientVpnEndpoint.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateClientVpnRoute.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateCoipCidr.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateCoipPool.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateCustomerGateway.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateDefaultSubnet.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateDefaultVpc.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateDelegateMacVolumeOwnershipTask.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateDhcpOptions.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateEgressOnlyInternetGateway.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateFleet.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateFlowLogs.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateFpgaImage.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateImage.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateImageUsageReport.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateInstanceConnectEndpoint.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateInstanceEventWindow.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateInstanceExportTask.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateInternetGateway.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateInterruptibleCapacityReservationAllocation.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateIpam.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateIpamExternalResourceVerificationToken.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateIpamPolicy.go is excluded by !**/vendor/**, !vendor/**
vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateIpamPool.go is excluded by !**/vendor/**, !vendor/**

📒 Files selected for processing (8)

go.mod
pkg/dns/aws/dns.go
pkg/dns/aws/dns_test.go
pkg/util/aws/shared_credentials_file.go
pkg/util/aws/shared_credentials_file_test.go
test/e2e/aws_util_test.go
test/e2e/lb_eip_test.go
test/e2e/operator_test.go

The aws-sdk-go library, used on DNS controller is deprecated and does not receive updates anymore. This change migrates to the new aws-sdk-go-v2, adding the new library and making the required changes to keep the code compatible

rikatz · 2026-05-13T22:05:44Z

@coderabbitai I want you to do a full review, line by line of the changes of this PR (ignore the vendor directory!) trying to find critical or major issues, possible breaking changes, misbehaviors and mistakes that can cause issues to users. Be critical on it, as this is a very sensible change

coderabbitai · 2026-05-13T22:08:28Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

I'll read all the relevant changed files to perform the thorough review you requested.

🏁 Script executed:

#!/bin/bash
echo "=== pkg/dns/aws/dns.go ==="
cat pkg/dns/aws/dns.go

echo ""
echo "=== pkg/dns/aws/dns_test.go ==="
cat pkg/dns/aws/dns_test.go

echo ""
echo "=== pkg/util/aws/shared_credentials_file.go ==="
cat pkg/util/aws/shared_credentials_file.go

echo ""
echo "=== pkg/util/aws/shared_credentials_file_test.go ==="
cat pkg/util/aws/shared_credentials_file_test.go

echo ""
echo "=== test/e2e/aws_util_test.go ==="
cat test/e2e/aws_util_test.go

echo ""
echo "=== test/e2e/lb_eip_test.go ==="
cat test/e2e/lb_eip_test.go

echo ""
echo "=== test/e2e/operator_test.go (relevant sections) ==="
cat test/e2e/operator_test.go

echo ""
echo "=== go.mod (AWS-related lines) ==="
grep -n "aws\|smithy\|jmespath" go.mod

Length of output: 269260

@rikatz — here is my thorough, line-by-line critical review of every changed file (vendor directory excluded). I've organized findings by severity.

🔴 Critical / Breaking Issues

1. GovCloud detection change: C2S/SC2S now use Alias records instead of CNAME

File: pkg/dns/aws/dns.go, updateRecord function

if isGovCloudRegion(m.route53.Options().Region) {
    // CNAME path
} else {
    // Alias A/AAAA path
}

In v1, the helper clientEndpointIsGovCloud checked the endpoint URL. In v2, it checks the region prefix (us-gov-). This means C2S (us-iso-*) and SC2S (us-isob-*) regions now fall into the Alias record path instead of the CNAME path. If the v1 code also used CNAME records for C2S/SC2S, this is a silent behavior regression that could break DNS in those environments.

Please verify whether the v1 clientEndpointIsGovCloud returned true for C2S/SC2S endpoint URLs, and if so, add us-iso and us-isob prefix detection to isGovCloudRegion (or create a separate helper).

2. `lookupZoneID`: Returns on first match without validating uniqueness

File: pkg/dns/aws/dns.go, lookupZoneID function (lines ~300–320)

for _, zone := range resp.ResourceTagMappingList {
    zoneARN, err := arn.Parse(aws.ToString(zone.ResourceARN))
    if err != nil {
        return "", fmt.Errorf("failed to parse hostedzone ARN %q: %v", ...)
    }
    return zoneIDFromResource(zoneARN.Resource)  // ← returns on first entry, ignores the rest
}

If arn.Parse or zoneIDFromResource fails for the first entry but a valid zone exists later in the list, the function returns an error rather than trying subsequent entries. This is a latent reliability issue, especially when paging through zones.

3. `SharedCredentialFile` as both credentials and config file

File: pkg/dns/aws/dns.go, NewProvider (lines ~220–225)

awsconfig.WithSharedCredentialsFiles([]string{config.SharedCredentialFile}),
awsconfig.WithSharedConfigFiles([]string{config.SharedCredentialFile}),

Both the shared credentials and the shared config are set to the same file path. In AWS SDK v2, WithSharedConfigFiles causes the SDK to also parse that file for config-file-format entries (e.g., role_arn, source_profile). If config.SharedCredentialFile is an empty string (zero value), the SDK will attempt to load "" as a file path, which may silently fail or load nothing. Add a guard:

if config.SharedCredentialFile != "" {
    loadOpts = append(loadOpts,
        awsconfig.WithSharedCredentialsFiles([]string{config.SharedCredentialFile}),
        awsconfig.WithSharedConfigFiles([]string{config.SharedCredentialFile}),
    )
}

🟠 Major Issues

4. `resolveEndpoint` is logging-only but tests assert against it

File: pkg/dns/aws/dns.go, resolveEndpoint + pkg/dns/aws/dns_test.go, Test_NewProvider

The resolveEndpoint function is explicitly documented as for "logging and test assertions" only — the SDK does its own per-request endpoint resolution internally. Tests in Test_NewProvider validate provider.elbEndpoint, provider.route53Endpoint, etc., which are derived entirely from resolveEndpoint.

This creates a false confidence risk: the tests can pass while the actual SDK resolves to a different endpoint in production. This is especially concerning for custom/override endpoints since those are passed as BaseEndpoint to the SDK options (line ~390), so they will take effect. But the default endpoint construction in resolveEndpoint is best-effort and not authoritative.

Consider adding a comment in the test making this limitation explicit, or testing with actual (mocked) SDK calls for at least one case.

5. GovCloud tagging endpoint override is fragile string matching

File: pkg/dns/aws/dns.go, NewProvider, custom endpoint processing

case TaggingService:
    ...
    if strings.Contains(ep.URL, "us-gov-east-1") {
        url = govCloudTaggingEndpoint  // → "https://tagging.us-gov-west-1.amazonaws.com"
    }
    tagEndpoint = url

If a user provides a custom tagging endpoint URL that happens to contain the string "us-gov-east-1" for any other reason (e.g., a proxy URL, a comment in a config), the URL is silently replaced with the hard-coded GovCloud west endpoint. This is brittle. A stronger check would be to test tagRegion == "us-gov-east-1" instead of URL substring matching.

6. `break` inside `switch default` case for EUSC — potentially confusing

File: pkg/dns/aws/dns.go, NewProvider, partition switch

default:
    if strings.HasPrefix(region, eusCloudRegionPrefix) {
        tagRegion = euscDeEast1RegionID
        r53Region = euscDeEast1RegionID
        break  // ← breaks the switch, not an inner loop
    }
    // AWS Standard Partition
    tagRegion = "us-east-1"
    r53Region = "us-east-1"

While this is functionally correct in Go (the break exits the switch), it is unusual and hard to read. A more idiomatic approach would be an else block:

default:
    if strings.HasPrefix(region, eusCloudRegionPrefix) {
        tagRegion = euscDeEast1RegionID
        r53Region = euscDeEast1RegionID
    } else {
        tagRegion = "us-east-1"
        r53Region = "us-east-1"
    }

7. `Test_partitionIDForRegion`: `eusc-de-east-1` maps to `"aws"` — no test for EUSC-specific behavior in `NewProvider` with default endpoints

File: pkg/dns/aws/dns_test.go

The test for Test_NewProvider only covers EUSC (eusc-de-east-1) with custom service endpoints. There is no test case for EUSC with default endpoints to verify that resolveEndpoint correctly computes route53.amazonaws.eu, tagging.eusc-de-east-1.amazonaws.eu, etc. This leaves a gap for regression if the default EUSC endpoint logic is broken.

8. Dual-stack not handled for GovCloud CNAME records

File: pkg/dns/aws/dns.go, updateRecord

if isGovCloudRegion(m.route53.Options().Region) {
    // Only creates a single CNAME record — no dual-stack handling
    record := r53types.ResourceRecord{Value: aws.String(target)}
    ...
} else {
    // Has dual-stack logic adding AAAA records
    if awsutil.IsDualStack(m.config.IPFamily) {
        changes = append(changes, ...)
    }
}

If a GovCloud cluster is configured with dual-stack IP family, no AAAA record is created. Whether this matches the GovCloud Route53 capabilities should be explicitly documented (or the CNAME block should add dual-stack handling if supported).

🟡 Minor Issues

9. `rand.Seed` is deprecated (Go ≥ 1.20)

File: test/e2e/lb_eip_test.go

rand.Seed(time.Now().UnixNano())

rand.Seed is deprecated since Go 1.20; the global source is now automatically seeded. This line is a no-op but should be removed.

10. `io/ioutil` import not cleaned up in `operator_test.go`

File: test/e2e/operator_test.go

"io/ioutil"

io/ioutil was deprecated in Go 1.16. While shared_credentials_file_test.go was updated to use os.ReadFile, operator_test.go still imports io/ioutil. This is a pre-existing issue that this PR could address since it's already touching the file.

11. `context.TODO()` used pervasively instead of propagated contexts

File: pkg/dns/aws/dns.go — all SDK calls

All SDK calls use context.TODO(). In v2, contexts support cancellation and timeout propagation. Consider accepting or propagating a context through the Provider methods (at minimum, Ensure, Delete, Replace) to allow proper timeout handling in production.

12. No validation that `resolveEndpoint` returns a non-empty string for all supported partitions

File: pkg/dns/aws/dns.go, resolveEndpoint

For the "Resource Groups Tagging API" service, the function doesn't handle aws-iso / aws-iso-b partitions (where tagging is disabled). It falls through and returns "". This is fine since those regions set enableTagging = false, but if that logic ever diverges, a silent empty endpoint string would be logged with no indication of the issue.

13. `cfgRoute53 := cfg` shallow copy — credential sharing risk

File: pkg/dns/aws/dns.go, NewProvider (~line 267)

cfgRoute53 := cfg
if config.RoleARN != "" {
    stsClient := sts.NewFromConfig(cfg)
    cfgRoute53.Credentials = aws.NewCredentialsCache(
        stscreds.NewAssumeRoleProvider(stsClient, config.RoleARN),
    )
}

cfg is an aws.Config struct value (not a pointer). The struct copy is shallow — fields that are interface types (like HTTPClient, Logger, ConfigSources) are shared between cfg and cfgRoute53. This is intentional and safe here since only Credentials is changed, but it's worth a comment explaining the intent (RoleARN only applies to Route53, not ELB/tagging).

Summary Table

#	Severity	File	Issue
1	🔴 Critical	`dns.go`	C2S/SC2S may now use Alias instead of CNAME (behavior change from v1)
2	🔴 Critical	`dns.go`	`lookupZoneID` returns error on first ARN parse failure without trying rest
3	🔴 Critical	`dns.go`	Empty `SharedCredentialFile` passed to SDK options without guard
4	🟠 Major	`dns.go` + test	`resolveEndpoint` is logging-only; tests don't verify actual SDK endpoints
5	🟠 Major	`dns.go`	GovCloud tagging URL override uses brittle substring matching
6	🟠 Major	`dns.go`	`break` in switch default for EUSC is non-idiomatic and confusing
7	🟠 Major	`dns_test.go`	Missing `Test_NewProvider` case for EUSC with default endpoints
8	🟠 Major	`dns.go`	No dual-stack (AAAA) support in GovCloud CNAME path
9	🟡 Minor	`lb_eip_test.go`	Deprecated `rand.Seed`
10	🟡 Minor	`operator_test.go`	Deprecated `io/ioutil` not cleaned up
11	🟡 Minor	`dns.go`	Pervasive `context.TODO()` prevents timeout/cancellation propagation
12	🟡 Minor	`dns.go`	`resolveEndpoint` silently returns `""` for tagging in iso partitions
13	🟡 Minor	`dns.go`	Shallow copy of `cfg` into `cfgRoute53` — intent should be commented

The most important item to verify before merging is Issue #1 (the GovCloud detection change affecting C2S/SC2S). The rest are important quality/robustness issues but lower risk for standard AWS deployments.

rikatz · 2026-05-13T22:32:32Z

@coderabbitai about your finding 1, The v1 Route53 endpoint for us-iso-east-1 would have been https://route53.c2s.ic.gov — which does NOT contain "us-gov". So clientEndpointIsGovCloud returned false for C2S/SC2S
in v1 too. Finding #1 is invalid. Can you comment on that?

coderabbitai · 2026-05-13T22:37:39Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for the v1 helper implementation in git history
git log --all --oneline | head -5

# Try to find clientEndpointIsGovCloud in current or previous commits
git log --all --full-history -- pkg/dns/aws/dns.go | head -10

# Check git history for the function
git show HEAD~1:pkg/dns/aws/dns.go 2>/dev/null | grep -A 5 "clientEndpointIsGovCloud" || echo "not in HEAD~1"

# Try to get the function from older commits
git log --all --oneline -- pkg/dns/aws/dns.go | head -5

Length of output: 988

🏁 Script executed:

#!/bin/bash
# Get the clientEndpointIsGovCloud function from the commit just before the migration
git show d16dd454:pkg/dns/aws/dns.go 2>/dev/null | grep -A 10 "clientEndpointIsGovCloud"

Length of output: 741

🏁 Script executed:

#!/bin/bash
# Find the value of govCloudRoute53Region in the pre-migration commit
git show d16dd454:pkg/dns/aws/dns.go 2>/dev/null | grep -n "govCloud"

Length of output: 392

@rikatz — you are absolutely right, and the code confirms it. In the pre-migration code:

govCloudRoute53Region = "us-gov"  // line 46

func clientEndpointIsGovCloud(clientInfo *metadata.ClientInfo) bool {
    return strings.Contains(clientInfo.Endpoint, govCloudRoute53Region)
}

The v1 function checked whether the Route53 endpoint URL contained "us-gov". The C2S endpoint (https://route53.c2s.ic.gov) does not contain that string, so clientEndpointIsGovCloud returned false for C2S/SC2S in v1 as well — meaning those regions were already using the Alias path, not the CNAME path.

The v2 isGovCloudRegion using strings.HasPrefix(region, "us-gov-") is therefore behaviorally equivalent to v1 for C2S/SC2S. Finding #1 is invalid — I apologize for the false alarm.

✏️ Learnings added

Learnt from: rikatz
URL: https://github.com/openshift/cluster-ingress-operator/pull/1445

Timestamp: 2026-05-13T22:37:38.380Z
Learning: In the cluster-ingress-operator (pkg/dns/aws/dns.go), C2S (us-iso-*) and SC2S (us-isob-*) regions were NOT treated as GovCloud in the v1 implementation. The v1 helper `clientEndpointIsGovCloud` checked for "us-gov" in the endpoint URL, and the C2S Route53 endpoint (https://route53.c2s.ic.gov) does not contain "us-gov". The v2 `isGovCloudRegion` using `strings.HasPrefix(region, "us-gov-")` preserves this same behavior. Both v1 and v2 use Alias records (not CNAME) for C2S/SC2S regions.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

rikatz · 2026-05-13T22:39:35Z

@coderabbitai some more assessment from your comments:

#: 2
  Finding: lookupZoneID returns on first match
  Verdict: Invalid
  Reason: Identical to v1 — by design, returns first matching zone.
  ────────────────────────────────────────
  #: 3
  Finding: Empty SharedCredentialFile guard
  Verdict: Invalid
  Reason: v1 also passed it without a guard. Pre-existing behavior.
  ────────────────────────────────────────
  #: 4
  Finding: Tests don't verify actual SDK endpoints
  Verdict: Invalid
  Reason: Inherent v2 limitation — no client.Endpoint field exists. Already documented.
  ────────────────────────────────────────
  #: 5
  Finding: GovCloud tagging URL substring matching
  Verdict: Invalid
  Reason: Same logic as v1, line 247.
  ────────────────────────────────────────
  #: 6
  Finding: break in switch default is confusing
  Verdict: Invalid
  Reason: Same pattern as v1, line 217.
#: 8
  Finding: No dual-stack in GovCloud CNAME
  Verdict: Invalid
  Reason: Same as v1.
#: 12
  Finding: resolveEndpoint returns "" for iso tagging
  Verdict: Invalid
  Reason: By design — enableTagging=false for those partitions.

comment on all of them and if so, do another review line by line checking if all of your concerns are addressed

coderabbitai · 2026-05-13T22:41:30Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
PRE_MIGRATION_COMMIT="d16dd454"

echo "=== Finding `#2`: lookupZoneID returns first match (v1 behavior) ==="
git show ${PRE_MIGRATION_COMMIT}:pkg/dns/aws/dns.go 2>/dev/null | grep -n -A 30 "func.*lookupZoneID"

echo ""
echo "=== Finding `#3`: Empty SharedCredentialFile guard (v1 behavior) ==="
git show ${PRE_MIGRATION_COMMIT}:pkg/dns/aws/dns.go 2>/dev/null | grep -n -B2 -A5 "SharedCredential"

echo ""
echo "=== Finding `#5`: GovCloud tagging URL substring match (v1 line ~247) ==="
git show ${PRE_MIGRATION_COMMIT}:pkg/dns/aws/dns.go 2>/dev/null | sed -n '235,265p'

echo ""
echo "=== Finding `#6`: break in switch default (v1 line ~217) ==="
git show ${PRE_MIGRATION_COMMIT}:pkg/dns/aws/dns.go 2>/dev/null | sed -n '205,235p'

echo ""
echo "=== Finding `#8`: Dual-stack in GovCloud CNAME path (v1 behavior) ==="
git show ${PRE_MIGRATION_COMMIT}:pkg/dns/aws/dns.go 2>/dev/null | grep -n -A 50 "func.*updateRecord"

echo ""
echo "=== Finding `#12`: resolveEndpoint iso tagging (v1 behavior) ==="
git show ${PRE_MIGRATION_COMMIT}:pkg/dns/aws/dns.go 2>/dev/null | grep -n -B2 -A5 "enableTagging"

echo ""
echo "=== Current dns.go: full file for fresh review ==="
cat pkg/dns/aws/dns.go

echo ""
echo "=== Current dns_test.go ==="
cat pkg/dns/aws/dns_test.go

echo ""
echo "=== test/e2e/lb_eip_test.go ==="
cat test/e2e/lb_eip_test.go

echo ""
echo "=== test/e2e/operator_test.go relevant imports ==="
head -60 test/e2e/operator_test.go

echo ""
echo "=== io/ioutil usages in operator_test.go ==="
grep -n "ioutil" test/e2e/operator_test.go | head -20

echo ""
echo "=== rand.Seed in test files ==="
grep -rn "rand\.Seed" test/

Length of output: 71906

@rikatz — all seven verdicts are confirmed by the v1 source. Here's my acknowledgment with evidence for each, followed by a fresh re-review.

Acknowledged Invalid Findings (with v1 evidence)

#2 — lookupZoneID returns on first match: Confirmed. The v1 callback (lines 371–382) does exactly the same: on the first matching entry it sets id and returns false to stop pagination. Identical by design. ✅

#3 — Empty SharedCredentialFile guard: Confirmed. v1 line 139: SharedConfigFiles: []string{config.SharedCredentialFile} — no guard, same as v2. Pre-existing behavior. ✅

#4 — Tests don't verify actual SDK endpoints: Confirmed. In v2 there is no client.Endpoint field to inspect. The resolveEndpoint helper + stored fields is the correct testing approach given the v2 architecture. ✅

#5 — GovCloud tagging URL substring match: Confirmed. v1 lines 247–250 contain the identical strings.Contains(ep.URL, "us-gov-east-1") logic and the same govCloudTaggingEndpoint fallback. ✅

#6 — break in switch default for EUSC: Confirmed. v1 lines 214–226 have the exact same pattern — break inside the default block to skip the standard us-east-1 assignment. ✅

#8 — No dual-stack in GovCloud CNAME path: Confirmed. v1 updateRecord lines 630–644 show the GovCloud branch creates a single CNAME only, with no dual-stack handling. Identical behavior preserved. ✅

#12 — resolveEndpoint returns "" for iso tagging: Confirmed. enableTagging = false for aws-iso/aws-iso-b partitions prevents the tags client from being created at all, so the empty string is intentional and unreachable in practice. ✅

#13 (self-correction) — cfgRoute53 shallow copy intent undocumented: Also invalid. The v2 code already has the comment: // When RoleARN is provided, make a copy of the Route 53 config and configure it to use RoleARN. // RoleARN is intended to only provide access to another account's Route 53 service, not for ELBs. The intent is documented. ✅

Fresh Line-by-Line Re-Review

With all 8 invalidated findings set aside, here is a clean pass on what genuinely remains.

🟠 Remaining Major Issue

#7 — No Test_NewProvider case for EUSC with default endpoints (still valid)

The test file covers EUSC only with custom service endpoints:

{
    name: "custom service endpoints, EUSC Brandenburg",
    config: Config{
        Region: "eusc-de-east-1",
        ServiceEndpoints: []ServiceEndpoint{...},
    },
    ...
}

There is no test case for Region: "eusc-de-east-1" with no ServiceEndpoints. This means the default-endpoint branch of resolveEndpoint for EUSC (e.g., https://route53.amazonaws.eu, https://tagging.eusc-de-east-1.amazonaws.eu) is untested. A missing or wrong format string in that branch would not be caught. Suggest adding:

{
    name: "default service endpoints, EUSC Brandenburg",
    config: Config{
        Region: "eusc-de-east-1",
    },
    expectedTaggingServiceEndpoint:       "https://tagging.eusc-de-east-1.amazonaws.eu",
    expectedElbServiceEndpointEndpoint:   "https://elasticloadbalancing.eusc-de-east-1.amazonaws.eu",
    expectedElbv2ServiceEndpointEndpoint: "https://elasticloadbalancing.eusc-de-east-1.amazonaws.eu",
    expectedRoute53ServiceEndpoint:       "https://route53.amazonaws.eu",
},

🟡 Remaining Minor Issues

#9 — No-op rand.Seed in lb_eip_test.go line 399

rand.Seed(time.Now().UnixNano())

This uses k8s.io/apimachinery/pkg/util/rand.Seed, which is a wrapper around math/rand.Seed. On Go ≥ 1.20, the global source is auto-seeded, making this a no-op. Harmless but can be removed.

#10 — Deprecated io/ioutil in operator_test.go (pre-existing, not introduced by this PR)

Lines 992, 2473, 2615, 2887, 3260, and 3333 still use ioutil.Discard / ioutil.ReadAll. This is pre-existing and this PR already touches the file for other reasons. Not a blocker, but worth a follow-up cleanup since io.Discard and io.ReadAll are the idiomatic replacements since Go 1.16.

#11 — Pervasive context.TODO() (pre-existing behavior parity)

All AWS API calls in dns.go and validateServiceEndpoints use context.TODO(). This matches v1 behavior and is not a regression. Noted for future improvement — propagating a context through Ensure/Delete/Replace would allow proper timeout/cancellation — but this is out of scope for a migration PR.

Summary of Remaining Items

#	Severity	Status	Description
7	🟠 Major	Still valid	Missing `Test_NewProvider` case for EUSC with default endpoints
9	🟡 Minor	Still valid	No-op `rand.Seed` (k8s wrapper) in `lb_eip_test.go`
10	🟡 Minor	Still valid (pre-existing)	Deprecated `io/ioutil` in `operator_test.go`
11	🟡 Minor	Still valid (pre-existing)	`context.TODO()` throughout; no context propagation

No new critical or breaking issues found in the fresh pass. The migration is behaviorally faithful to v1. The only concrete code suggestion I'd flag before merging is #7 (EUSC default-endpoint test case).

✏️ Learnings added

Learnt from: rikatz
URL: https://github.com/openshift/cluster-ingress-operator/pull/1445

Timestamp: 2026-05-13T22:41:29.025Z
Learning: In cluster-ingress-operator (pkg/dns/aws/dns.go), findings `#2`–#6, `#8`, `#12` from the aws-sdk-go v1→v2 migration review (PR `#1445`) are invalid: all represent intentional behavior parity with v1. Specifically: lookupZoneID returning on first match, no empty SharedCredentialFile guard, GovCloud tagging endpoint URL substring matching, break in switch default for EUSC, and no dual-stack in GovCloud CNAME path — all are direct ports of v1 behavior.

Learnt from: rikatz
URL: https://github.com/openshift/cluster-ingress-operator/pull/1445

Timestamp: 2026-05-13T22:41:29.025Z
Learning: In cluster-ingress-operator (pkg/dns/aws/dns.go), the v2 migration (PR `#1445`) already documents the cfgRoute53 shallow copy intent with a comment: "When RoleARN is provided, make a copy of the Route 53 config and configure it to use RoleARN. RoleARN is intended to only provide access to another account's Route 53 service, not for ELBs."

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

rikatz · 2026-05-14T12:03:48Z

@coderabbitai do the same review I have asked you again, line by line, critical and major findings, adversarial (give me reasons not to merge, if you have some).

Also verify why ci/prow/e2e-aws-operator execution failed before, and if it is related with this PR

coderabbitai · 2026-05-14T12:10:10Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

I'll do the fresh adversarial re-review and simultaneously investigate the CI failure.

🏁 Script executed:

#!/bin/bash
echo "=== Full dns.go ==="
cat pkg/dns/aws/dns.go

echo ""
echo "=== Full dns_test.go ==="
cat pkg/dns/aws/dns_test.go

echo ""
echo "=== test/e2e/aws_util_test.go ==="
cat test/e2e/aws_util_test.go

echo ""
echo "=== test/e2e/lb_eip_test.go ==="
cat test/e2e/lb_eip_test.go

echo ""
echo "=== test/e2e/operator_test.go - EC2/EIP related sections ==="
grep -n "ec2\|EIP\|eip\|assertEIPAllocation\|createEC2\|getPublicSubnet\|getVPCId\|createAWSEIP" test/e2e/operator_test.go | head -60

echo ""
echo "=== go.mod AWS entries ==="
grep -n "aws\|smithy\|jmespath" go.mod

echo ""
echo "=== Check PR CI status ==="
gh pr checks 1445 --repo openshift/cluster-ingress-operator 2>/dev/null | head -60

echo ""
echo "=== Check PR reviews and comments for CI failure ==="
gh pr view 1445 --repo openshift/cluster-ingress-operator --json statusCheckRollup 2>/dev/null | head -200

echo ""
echo "=== List check runs ==="
gh api repos/openshift/cluster-ingress-operator/commits/$(gh pr view 1445 --repo openshift/cluster-ingress-operator --json headRefOid --jq '.headRefOid')/check-runs 2>/dev/null | python3 -c "
import json, sys
data = json.load(sys.stdin)
for run in data.get('check_runs', []):
    print(f\"Name: {run['name']}\")
    print(f\"Status: {run['status']}\")
    print(f\"Conclusion: {run.get('conclusion', 'N/A')}\")
    print(f\"URL: {run.get('html_url', 'N/A')}\")
    print('---')
" 2>/dev/null | head -200

Length of output: 81698

All CI checks are currently PENDING (re-triggered just now at ~12:01 UTC). Let me check what the previous failure was, and simultaneously verify the service/signin dependency.

Let me check the prow job history for the previous failed run.

🏁 Script executed:

#!/bin/bash
# Check prow job history for previous runs on this PR
# The current run URL is: https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_cluster-ingress-operator/1445/pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-operator/2054894919390072832

# List all PR comments on PR 1445 to find any previous CI failure mentions
gh pr view 1445 --repo openshift/cluster-ingress-operator --comments --json comments 2>/dev/null | python3 -c "
import json, sys
data = json.load(sys.stdin)
for comment in data.get('comments', []):
    if 'prow' in comment.get('body','').lower() or 'e2e' in comment.get('body','').lower() or 'fail' in comment.get('body','').lower() or 'ci' in comment.get('body','').lower():
        print('Author:', comment['author']['login'])
        print('Body:', comment['body'][:600])
        print('---')
" 2>/dev/null

echo "=== Check issue events for status contexts ==="
# Check previous commit statuses on the PR
PREV_COMMIT=$(gh api repos/openshift/cluster-ingress-operator/commits/$(gh pr view 1445 --repo openshift/cluster-ingress-operator --json headRefOid --jq '.headRefOid')/statuses 2>/dev/null | python3 -c "
import json, sys
data = json.load(sys.stdin)
for status in data[:30]:
    if 'e2e' in status.get('context','').lower():
        print(f\"Context: {status['context']}\")
        print(f\"State: {status['state']}\")
        print(f\"Description: {status.get('description','')}\")
        print(f\"URL: {status.get('target_url','')}\")
        print('---')
" 2>/dev/null)
echo "$PREV_COMMIT"

echo "=== Try prow job history API for previous runs ==="
# Try to find the job history for this specific PR
curl -s "https://prow.ci.openshift.org/job-history/gs/test-platform-results/pr-logs/pull/openshift_cluster-ingress-operator/1445/pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-operator" 2>/dev/null | grep -o '"buildID":"[0-9]*"' | head -5 || echo "Cannot access prow history directly"

# Check the full status list sorted by date
gh api repos/openshift/cluster-ingress-operator/commits/$(gh pr view 1445 --repo openshift/cluster-ingress-operator --json headRefOid --jq '.headRefOid')/statuses 2>/dev/null | python3 -c "
import json, sys
data = json.load(sys.stdin)
# Find failure states
for status in data:
    if status.get('state') in ('failure', 'error'):
        print(f\"Context: {status['context']}\")
        print(f\"State: {status['state']}\")
        print(f\"Description: {status.get('description','')}\")
        print(f\"URL: {status.get('target_url','')}\")
        print(f\"Updated: {status.get('updated_at','')}\")
        print('---')
" 2>/dev/null | head -100

Length of output: 8730

Found the previous failed run. Let me fetch its logs to diagnose the failure.

Let me fetch the specific prow job logs directly.

🏁 Script executed:

#!/bin/bash
# Fetch the failed prow job artifacts to understand what failed
FAILED_JOB_URL="https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_cluster-ingress-operator/1445/pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-operator/2054683983760855040"

# Try to get the junit results
JOB_BASE="gs://test-platform-results/pr-logs/pull/openshift_cluster-ingress-operator/1445/pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-operator/2054683983760855040"

# Try fetching via the spyglass/artifacts API
curl -s "https://prow.ci.openshift.org/log?job=pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-operator&id=2054683983760855040" 2>/dev/null | tail -100 || echo "Cannot access log directly"

# Try the artifacts endpoint  
curl -s "https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/openshift_cluster-ingress-operator/1445/pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-operator/2054683983760855040/" 2>/dev/null | grep -o 'href="[^"]*"' | head -30 || echo "Cannot list artifacts"

# Try finished.json to get overall status/failure info
curl -s "https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/openshift_cluster-ingress-operator/1445/pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-operator/2054683983760855040/finished.json" 2>/dev/null || echo "Cannot access finished.json"

echo "=== Try prowjob info ==="
curl -s "https://prow.ci.openshift.org/prowjob?prowjob=2054683983760855040" 2>/dev/null | python3 -c "
import json, sys
try:
    data = json.load(sys.stdin)
    print(json.dumps(data.get('status', {}), indent=2))
except:
    content = sys.stdin.read()
    print(content[:2000])
" 2>/dev/null || true

Length of output: 2737

Good - confirmed FAILURE on the previous commit. Now let me get the build log and test results.

🏁 Script executed:

#!/bin/bash
BASE="https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/openshift_cluster-ingress-operator/1445/pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-operator/2054683983760855040"

echo "=== build-log.txt (last 200 lines) ==="
curl -s "$BASE/build-log.txt" 2>/dev/null | tail -200

echo ""
echo "=== List artifacts directory ==="
curl -s "$BASE/artifacts/" 2>/dev/null | grep -o 'href="[^"]*"' | head -30

echo ""
echo "=== prowjob_junit.xml ==="
curl -s "$BASE/prowjob_junit.xml" 2>/dev/null | head -100

Length of output: 22299

Found the failing test: TestManagedDNSToUnmanagedDNSIngressController. This directly touches the DNS delete path. Let me get the failure details.

🏁 Script executed:

#!/bin/bash
BASE="https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/openshift_cluster-ingress-operator/1445/pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-operator/2054683983760855040"

echo "=== junit_operator.xml (failures only) ==="
curl -s "$BASE/artifacts/junit_operator.xml" 2>/dev/null | python3 -c "
import sys, xml.etree.ElementTree as ET
content = sys.stdin.read()
try:
    root = ET.fromstring(content)
    for tc in root.iter('testcase'):
        failures = tc.findall('failure')
        errors = tc.findall('error')
        if failures or errors:
            print('TEST:', tc.get('name'))
            print('CLASSNAME:', tc.get('classname', ''))
            print('TIME:', tc.get('time'))
            for f in failures:
                print('FAILURE:', f.get('message', ''))
                print('BODY:', (f.text or '')[:2000])
            for e in errors:
                print('ERROR:', e.get('message', ''))
                print('BODY:', (e.text or '')[:2000])
            print('---')
except Exception as ex:
    print('Parse error:', ex)
    print(content[:3000])
" 2>/dev/null

echo ""
echo "=== Look for test-specific e2e log ==="
curl -s "$BASE/artifacts/e2e-aws-operator/" 2>/dev/null | grep -o 'href="[^"]*"' | head -30

echo ""
echo "=== Check e2e-aws-operator-test directory ==="
curl -s "$BASE/artifacts/e2e-aws-operator/e2e-aws-operator-test/" 2>/dev/null | grep -o 'href="[^"]*"' | head -30

Length of output: 12999

Let me get the detailed test artifacts for the specific failure.

🏁 Script executed:

#!/bin/bash
BASE="https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/openshift_cluster-ingress-operator/1445/pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-operator/2054683983760855040"

echo "=== List e2e-aws-operator/test directory ==="
curl -s "$BASE/artifacts/e2e-aws-operator/test/" 2>/dev/null | grep -o 'href="[^"]*"' | head -30

echo ""
echo "=== Try to get the e2e junit results from artifacts ==="
# Try the test directory for junit files
curl -s "$BASE/artifacts/e2e-aws-operator/" 2>/dev/null | grep -o 'href="[^"]*junit[^"]*"' | head -10

# Also try the direct test log
echo ""
echo "=== Check build-resources ==="
curl -s "$BASE/artifacts/build-resources/" 2>/dev/null | grep -o 'href="[^"]*"' | head -20

# Try to get the operator log from the cluster
echo ""
echo "=== Check gather-extra directory ==="
curl -s "$BASE/artifacts/e2e-aws-operator/gather-extra/" 2>/dev/null | grep -o 'href="[^"]*"' | head -20

# Look at the full build log specifically around the failing test
echo ""
echo "=== build-log.txt - TestManagedDNSToUnmanaged section ==="
curl -s "$BASE/build-log.txt" 2>/dev/null | grep -A 50 -B 5 "TestManagedDNSToUnmanagedDNSIngressController"

Length of output: 33121

@rikatz — here is the comprehensive fresh adversarial re-review, followed by the CI failure diagnosis.

Fresh Adversarial Re-Review

✅ Correction from previous round: Finding `#7` is also invalid

I previously said there was no Test_NewProvider test case for EUSC with default endpoints. I was wrong — "default service endpoints, EUSC Brandenburg" IS present in the test table, covering all four expected default endpoints. Finding #7 is invalid.

🔴 Reason Not to Merge `#1` — CI failure is DNS-specific and likely PR-related

This is the most important finding. The previous CI run (job 2054683983760855040) failed on a single test:

--- FAIL: TestAll/parallel/TestManagedDNSToUnmanagedDNSIngressController (138.28s)

The specific error from the build log:

unmanaged_dns_test.go:187: DNSRecord zone expected to have status=Unknown but got status=True

Test flow breakdown:

Create IngressController with ManagedLoadBalancerDNS → ✅ DNS record created, connectivity verified
Switch IC to dnsManagementPolicy=Unmanaged
Wait for stable IngressController conditions → ✅ waited
Check DNSRecord zone condition for the internal zone (tag-based lookup) → FAILED: still True, expected Unknown

Why this is suspicious:

TestUnmanagedDNSToManagedDNSIngressController (the reverse direction: unmanaged→managed, DNS CREATE path) passed at 138.44s
Only the DELETE/disownment path (managed→unmanaged) failed
This PR rewrites the entire DNS provider including the Delete → change → updateRecord call chain

Possible root causes:

The v2 SDK's Delete call silently fails or returns a non-fatal error while the DNS record still exists in Route53, causing the reconciler to still set status=True on the next reconcile cycle before it can set it to Unknown
A timing issue: the reconciler sets the condition to Unknown asynchronously, but the delete in v2 takes longer than in v1 (e.g., due to SDK retry/backoff differences), leaving a window where the old True status is still observed

This test failed on the previous commit (6d8c121f) — the retest on the current head (10bbe5fa) is still running. The result of that retest should be treated as a gate for merging. If it fails again, the delete path needs to be investigated.

🟠 Reason Not to Merge `#2` — `updateRecord` delete error detection uses string matching instead of error code

if action == string(deleteAction) {
    var apiErr smithy.APIError
    if errors.As(err, &apiErr) {
        if strings.Contains(apiErr.ErrorMessage(), "not found") {
            log.Info("record not found", ...)
            return nil  // ← silently swallows the error
        }
    }
}

AWS SDK v2's smithy.APIError exposes both ErrorCode() and ErrorMessage(). For a non-existent record during deletion, Route53 returns:

Code: InvalidChangeBatch
Message: "Tried to delete resource record set [...] but it was not found"

The check relies on the message substring "not found" rather than the error code. This is fragile: if any other Route53 error happens to contain "not found" in its message, deletion errors would be silently swallowed and the record would appear deleted when it wasn't. Importantly, the condition update in the reconciler depends on Delete() returning nil to consider the record successfully deleted. A stronger implementation:

var apiErr smithy.APIError
if errors.As(err, &apiErr) &&
    apiErr.ErrorCode() == "InvalidChangeBatch" &&
    strings.Contains(apiErr.ErrorMessage(), "not found") {
    // ...
}

This is pre-existing v1 behavior but v2's type system makes the safer check trivial. Given that the CI failure is on the delete path, this is worth tightening now.

🟡 Minor Issues (not blockers, but adversarial completeness)

3. `lookupZoneIDWithoutResourceTagging` — `ListTagsForResources` with empty slice

If the first page of ListHostedZones returns 0 zones, resourceIds is []string{} and ListTagsForResources is called with an empty ResourceIds. The Route53 API may return an error for an empty list. This is v1 parity and only affects accounts with 0 hosted zones, but it is a potential panic-free crash path in unusual environments (e.g., CI clusters with no zones).

4. `service/signin v1.0.11` in `go.mod` is unusual

github.com/aws/aws-sdk-go-v2/service/signin v1.0.11 // indirect

This is the AWS IAM Identity Center Sign-in API — not typically needed for DNS/ELB operations. It is likely a transitive dependency from service/sso or service/ssooidc (used for SSO credential chaining). It is worth verifying that this was pulled in intentionally and not as a stale/erroneous transitive dep.

5. `govCloudRoute53Region` constant dual use is easy to misread

The constant govCloudRoute53Region = "us-gov" is used both as:

A URL fragment: fmt.Sprintf("https://route53.%s.amazonaws.com", govCloudRoute53Region) → "https://route53.us-gov.amazonaws.com" ✓
A doc constant (with comments referencing the endpoint)

But in NewProvider, the GovCloud Route53 region is set to "us-gov-west-1", not the constant. A future developer editing the resolveEndpoint function might assume govCloudRoute53Region is a full region ID and use it as o.Region — which would be wrong. A comment or rename (govCloudRoute53EndpointFragment) would disambiguate.

CI Failure Verdict


Failed test	`TestManagedDNSToUnmanagedDNSIngressController`
Error	`DNSRecord zone expected to have status=Unknown but got status=True`
Path exercised	DNS `Delete` path (managed→unmanaged transition)
PR-related?	Suspicious — this test exclusively exercises the DNS delete path which this PR rewrites, and the inverse test (unmanaged→managed, DNS create) passed.
Verdict	Do not merge until the re-triggered CI run passes. If it fails again, instrument the `Delete`/`updateRecord` code path with more diagnostic logging and verify the error detection logic (Finding `#2` above).

✏️ Learnings added

Learnt from: rikatz
URL: https://github.com/openshift/cluster-ingress-operator/pull/1445

Timestamp: 2026-05-14T12:10:08.890Z
Learning: In the cluster-ingress-operator (pkg/dns/aws/dns.go) v2 migration PR `#1445`, the `TestManagedDNSToUnmanagedDNSIngressController` e2e test failed on a previous commit (6d8c121f) with error "DNSRecord zone expected to have status=Unknown but got status=True" - meaning after switching to UnmanagedLoadBalancerDNS, the internal zone's DNSRecord condition was still True instead of Unknown. This test exercises the DNS DELETE/disownment path which was rewritten in the v2 migration.

Learnt from: rikatz
URL: https://github.com/openshift/cluster-ingress-operator/pull/1445

Timestamp: 2026-05-14T12:10:08.890Z
Learning: In cluster-ingress-operator pkg/dns/aws/dns.go (PR `#1445`), Test_NewProvider does include a "default service endpoints, EUSC Brandenburg" test case with no ServiceEndpoints configured - verifying default endpoint resolution for eusc-de-east-1. Finding that this test was missing was incorrect.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

rikatz · 2026-05-14T16:05:48Z

The test that I care right now is e2e-aws-operator, which is flaky by different reasons apparently, so let me try again

/test e2e-aws-operator

rikatz · 2026-05-15T11:44:50Z

/retest-required

rikatz · 2026-05-15T11:46:43Z

@coderabbitai the last test passed on aws-operator pipeline. I want you to do a new full adversarial review, line by line of the changes and the proposal, but keep yourself in the current context of the change: bumping to aws-sdk-go-v2. Is there any good reason not to merge this?

coderabbitai · 2026-05-15T11:48:47Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== Full dns.go ==="
cat pkg/dns/aws/dns.go

echo ""
echo "=== Full dns_test.go ==="
cat pkg/dns/aws/dns_test.go

echo ""
echo "=== pkg/util/aws/shared_credentials_file.go ==="
cat pkg/util/aws/shared_credentials_file.go

echo ""
echo "=== pkg/util/aws/shared_credentials_file_test.go ==="
cat pkg/util/aws/shared_credentials_file_test.go

echo ""
echo "=== test/e2e/aws_util_test.go ==="
cat test/e2e/aws_util_test.go

echo ""
echo "=== test/e2e/lb_eip_test.go ==="
cat test/e2e/lb_eip_test.go

echo ""
echo "=== test/e2e/operator_test.go: imports and assertEIPAllocationDeleted, assertEIPAllocationExists ==="
head -80 test/e2e/operator_test.go
grep -n "assertEIPAlloc\|createEC2\|getPublicSubnet\|getVPCId\|io/ioutil\|ioutil\." test/e2e/operator_test.go | head -40

echo ""
echo "=== go.mod AWS/smithy lines ==="
grep -n "aws\|smithy\|jmespath\|signin" go.mod

echo ""
echo "=== Check go.sum for service/signin to see what pulled it in ==="
grep "service/signin" go.sum | head -5

echo ""
echo "=== Check vendor for service/signin usage ==="
grep -r "service/signin" vendor/ --include="*.go" -l 2>/dev/null | head -5

echo ""
echo "=== Check imports of smithy in dns.go ==="
grep -n "smithy\|errors\." pkg/dns/aws/dns.go | head -20

echo ""
echo "=== Check ChangeResourceRecordSets call in dns.go ==="
grep -n -A 5 -B 5 "ChangeResourceRecordSets\|InvalidChangeBatch\|not found\|smithy" pkg/dns/aws/dns.go | head -60

echo ""
echo "=== Check ListTagsForResources call ==="
grep -n -A 10 "ListTagsForResources\|resourceIds" pkg/dns/aws/dns.go | head -40

echo ""
echo "=== Check GetResources paginator usage ==="
grep -n -A 20 "NewGetResourcesPaginator\|GetResourcesInput" pkg/dns/aws/dns.go | head -60

echo ""
echo "=== Check pagination max results / page sizes ==="
grep -n "MaxResult\|MaxItems\|maxItems\|MaxRecords\|pageSize\|PaginationToken\|NextToken\|NextPage\|HasMorePages" pkg/dns/aws/dns.go | head -30

echo ""
echo "=== Check DescribeLoadBalancers paginators ==="
grep -n -A 15 "NewDescribeLoadBalancersPaginator" pkg/dns/aws/dns.go | head -60

echo ""
echo "=== Check all error returns in Delete path ==="
grep -n -A 5 "deleteAction\|action == string" pkg/dns/aws/dns.go | head -40

Length of output: 84378

@rikatz — here is the final adversarial pass with the full source in hand. Short answer: no, there is no good reason to block this merge. Here is the complete reasoning.

Final Adversarial Line-by-Line Review

🟢 Items Previously Flagged — Confirmed Resolved or Invalid

Finding	Status
`#1` C2S/SC2S CNAME vs Alias	✅ Invalid — confirmed v1 parity
`#2` `lookupZoneID` first-match return	✅ Invalid — v1 parity, by design
`#3` Empty `SharedCredentialFile` guard	✅ Invalid — v1 parity; SDK gracefully skips unreadable files
`#4` `resolveEndpoint` testing limitation	✅ Invalid — inherent v2 architecture
`#5` GovCloud tagging URL substring	✅ Invalid — v1 parity
`#6` `break` in switch default	✅ Invalid — v1 parity
`#7` EUSC default-endpoint test case	✅ Invalid — test case is present in `Test_NewProvider`
`#8` No dual-stack in GovCloud CNAME	✅ Invalid — v1 parity
`#9` `rand.Seed` no-op	✅ Already fixed — `lb_eip_test.go` now uses `crypto/rand` and the `rand.Seed` call is gone
`#10` `io/ioutil` in `operator_test.go`	✅ Already fixed — import is now `"io"` throughout
`#12` `resolveEndpoint` empty for iso	✅ Invalid — `enableTagging=false`, unreachable
`#13` Shallow copy comment	✅ Already present in the code

🔍 New Pass — Items Examined Fresh

1. smithy.APIError message matching in delete path (Line 809)

if strings.Contains(apiErr.ErrorMessage(), "not found") {

This is unchanged v1 behavior. The v2 type system makes a code-based check trivial to add (apiErr.ErrorCode() == "InvalidChangeBatch"), but since the CI passed including the delete-path test (TestManagedDNSToUnmanagedDNSIngressController), this is not a regression and is out of scope for a migration PR. Worth a follow-up issue if desired.

2. lookupZoneIDWithoutResourceTagging empty-slice edge case

If an account has zero hosted zones, the paginator fires once, resp.HostedZones is empty, resourceIds is []string{}, and ListTagsForResources is called with an empty list (which Route53 rejects). This is verbatim v1 behavior and only possible on an account with no hosted zones — not a real scenario for a running OpenShift cluster. Not a blocker.

3. aliasTarget pointer shared between A and AAAA changes (dual-stack)

aliasTarget := &r53types.AliasTarget{...}
changes := []r53types.Change{{..., AliasTarget: aliasTarget}}
if awsutil.IsDualStack(...) {
    changes = append(changes, r53types.Change{{..., AliasTarget: aliasTarget}})
}

Both changes point to the same *AliasTarget. The SDK serializes but does not mutate the input struct, so this is safe. Matches the intended behavior (same ELB target for A and AAAA).

4. service/signin v1.0.11 indirect dependency in go.mod

No file in vendor directly imports this package. It is a transitive dependency of service/sso/service/ssooidc (used for IAM Identity Center SSO credential chaining, which is a standard AWS SDK v2 credential source). Harmless.

5. validateServiceEndpoints — GetResources called with no filters

tagInput := resourcegroupstaggingapi.GetResourcesInput{TagsPerPage: aws.Int32(100)}

This is explicitly just a connectivity check (the result is discarded). Fetching up to 100 resources once at startup to confirm the tagging endpoint is reachable is correct and cheap. Fine.

6. Test coverage completeness

Test_NewProvider covers: us-east-1 (standard), eu-west-1-equivalent via standard partition, eusc-de-east-1 default+custom, us-gov-east-1 (GovCloud), us-iso-east-1 (C2S), us-isob-east-1 (SC2S), cn-north-1 (China). Unit tests for partitionIDForRegion and isGovCloudRegion cover the entire partition space. Test_zoneMatchesTags and Test_zoneIDFromResource cover the tag/zone lookup helpers.

7. go.mod module list

All added modules are purposeful: core v2 (aws-sdk-go-v2), config/credentials, EC2 (e2e tests), ELB/ELBv2, resourcegroupstaggingapi, route53, STS, smithy-go. jmespath correctly removed (v1 dependency). No unexpected additions.

Verdict

The migration is behaviorally faithful to v1 across all partitions, test-covered for all supported region types, passing CI (including the previously failed delete-path test), and the code is clean and well-commented. There is no good reason not to merge.

The only two items worth noting as optional follow-ups (not blockers):

Tighten the delete error check to also match on apiErr.ErrorCode() == "InvalidChangeBatch" in addition to the message substring.
Guard lookupZoneIDWithoutResourceTagging against the zero-hosted-zone empty-slice case.

Neither is introduced by this PR and neither affects any real deployment scenario.

rikatz · 2026-05-15T17:42:25Z

/retest-required
🤔 if those are really related to AWS, or to this change, or what (concerned)

rikatz · 2026-05-15T17:42:45Z

@coderabbitai verify if the last pipeline failures at #1445 (comment) are related to this PR changes

coderabbitai · 2026-05-15T17:44:16Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== Fetch the specific comment 4461186058 ==="
gh api repos/openshift/cluster-ingress-operator/issues/comments/4461186058 2>/dev/null | python3 -c "
import json, sys
data = json.load(sys.stdin)
print('Author:', data.get('user', {}).get('login'))
print('Created:', data.get('created_at'))
print('Body:')
print(data.get('body', ''))
" 2>/dev/null

echo ""
echo "=== Get all PR comments to find context around that comment ==="
gh api repos/openshift/cluster-ingress-operator/issues/1445/comments 2>/dev/null | python3 -c "
import json, sys
data = json.load(sys.stdin)
for comment in data:
    print(f\"ID: {comment['id']} | Author: {comment['user']['login']} | Created: {comment['created_at']}\")
    print(comment['body'][:300])
    print('---')
" 2>/dev/null

echo ""
echo "=== Get current CI check statuses ==="
HEAD_SHA=$(gh pr view 1445 --repo openshift/cluster-ingress-operator --json headRefOid --jq '.headRefOid' 2>/dev/null)
echo "HEAD SHA: $HEAD_SHA"

gh api repos/openshift/cluster-ingress-operator/commits/${HEAD_SHA}/check-runs 2>/dev/null | python3 -c "
import json, sys
data = json.load(sys.stdin)
for run in data.get('check_runs', []):
    print(f\"Name: {run['name']}\")
    print(f\"Status: {run['status']}\")
    print(f\"Conclusion: {run.get('conclusion', 'N/A')}\")
    print(f\"URL: {run.get('html_url', 'N/A')}\")
    print('---')
" 2>/dev/null

echo ""
echo "=== Get commit statuses ==="
gh api repos/openshift/cluster-ingress-operator/commits/${HEAD_SHA}/statuses 2>/dev/null | python3 -c "
import json, sys
data = json.load(sys.stdin)
for status in data[:30]:
    print(f\"Context: {status['context']}\")
    print(f\"State: {status['state']}\")
    print(f\"Description: {status.get('description','')}\")
    print(f\"URL: {status.get('target_url','')}\")
    print(f\"Updated: {status.get('updated_at','')}\")
    print('---')
" 2>/dev/null

Length of output: 18389

Let me fetch the actual failure logs for the three OVN jobs to diagnose the root cause.

🏁 Script executed:

#!/bin/bash
GCS_BASE="https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/openshift_cluster-ingress-operator/1445"

echo "=== e2e-aws-ovn build-log.txt (last 150 lines) ==="
curl -s "$GCS_BASE/pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-ovn/2055253150222258176/build-log.txt" 2>/dev/null | tail -150

echo ""
echo "=== e2e-aws-ovn-serial-1of2 build-log.txt (last 150 lines) ==="
curl -s "$GCS_BASE/pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-ovn-serial-1of2/2055253150251618304/build-log.txt" 2>/dev/null | tail -150

echo ""
echo "=== e2e-aws-ovn-serial-2of2 build-log.txt (last 150 lines) ==="
curl -s "$GCS_BASE/pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-ovn-serial-2of2/2055253150285172736/build-log.txt" 2>/dev/null | tail -150

echo ""
echo "=== e2e-aws-ovn finished.json ==="
curl -s "$GCS_BASE/pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-ovn/2055253150222258176/finished.json" 2>/dev/null

echo ""
echo "=== e2e-aws-ovn-serial-1of2 finished.json ==="
curl -s "$GCS_BASE/pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-ovn-serial-1of2/2055253150251618304/finished.json" 2>/dev/null

echo ""
echo "=== e2e-aws-ovn-serial-2of2 finished.json ==="
curl -s "$GCS_BASE/pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-ovn-serial-2of2/2055253150285172736/finished.json" 2>/dev/null

echo ""
echo "=== Also check PREVIOUS run of e2e-aws-ovn (to compare) ==="
curl -s "$GCS_BASE/pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-ovn/2054894923580182528/build-log.txt" 2>/dev/null | tail -80

echo ""
echo "=== junit xml for e2e-aws-ovn ==="
curl -s "$GCS_BASE/pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-ovn/2055253150222258176/artifacts/junit_operator.xml" 2>/dev/null | python3 -c "
import sys, xml.etree.ElementTree as ET
content = sys.stdin.read()
try:
    root = ET.fromstring(content)
    for tc in root.iter('testcase'):
        failures = tc.findall('failure')
        errors = tc.findall('error')
        if failures or errors:
            print('TEST:', tc.get('name'))
            for f in failures:
                print('FAILURE:', f.get('message','')[:500])
                print('BODY:', (f.text or '')[:1000])
            for e in errors:
                print('ERROR:', e.get('message','')[:500])
                print('BODY:', (e.text or '')[:1000])
            print('---')
except Exception as ex:
    print('Parse error:', ex)
    print(content[:2000])
" 2>/dev/null

echo ""
echo "=== Check whether master branch is broken (recent CI runs without this PR) ==="
# Check the most recent master commit that's NOT this PR to see if OVN is also failing there
curl -s "https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/openshift_cluster-ingress-operator/1445/pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-ovn/2055253150222258176/artifacts/" 2>/dev/null | grep -o 'href="[^"]*"' | head -20

Length of output: 54174

@rikatz — I've dug into the logs for all three failing jobs. None of them are related to this PR's changes. Here is the breakdown:

`ci/prow/e2e-aws-ovn` — CI Infrastructure Failure ❌ (not PR-related)

This job never reached the test phase. It failed during the pre-install nodes-readiness step with a pure kubelet/CI infrastructure issue:

kubelet: Failed to create pod sandbox: rpc error: code = DeadlineExceeded
         desc = context deadline exceeded  [10x]
kubelet: ...stream terminated by RST_STREAM with error code: CANCEL  [4x]

The CI node hosting the prow pod couldn't create pod sandboxes. No test code ran at all. This is an OpenShift CI cluster infrastructure problem, entirely unrelated to this PR.

`ci/prow/e2e-aws-ovn-serial-1of2` — Unrelated sig-cli Test Failure ❌ (not PR-related)

The cluster installed successfully and tests ran. The single blocking failure was:

[Serial][sig-cli] oc adm upgrade recommend
  When the update service has conditional recommendations
  runs successfully with conditional recommendations to the --version target

This is a sig-cli / OTA (Over-The-Air updates) conformance test about oc adm upgrade conditional recommendations. It has no connection to cluster-ingress-operator, DNS, Route53, ELB, or the AWS SDK.

`ci/prow/e2e-aws-ovn-serial-2of2` — Platform MonitorTest Failure ❌ (not PR-related)

Failed with failed due to a MonitorTest failure — a platform-level monitoring invariant (not an ingress test). Notably, the mass failure threshold check passed (3 failures, threshold: 10), meaning overall test health was fine; it was a monitor invariant that tripped the suite exit code. The same pattern was present in the previous run of this job (2026-05-14), confirming this is recurring OVN/platform-level CI noise.

Summary

Job	Failure Cause	PR-related?
`e2e-aws-ovn`	Kubelet pod sandbox creation failed — CI infra issue	❌ No
`e2e-aws-ovn-serial-1of2`	`oc adm upgrade recommend` test — sig-cli/OTA test	❌ No
`e2e-aws-ovn-serial-2of2`	Platform MonitorTest invariant failure	❌ No

Critically, the job that directly exercises your changes — ci/prow/e2e-aws-operator — passed ✅ (along with e2e-azure-operator, e2e-gcp-operator, e2e-aws-operator-techpreview, e2e-aws-ovn-upgrade, and 4 others). The /retest-required you just triggered will rerun the three OVN jobs, and given the failure patterns, they are expected to be CI environment flakiness. You should not be concerned about this PR's code being the cause.

jcmoraisjr · 2026-05-20T14:38:44Z

/assign

rikatz · 2026-05-20T17:25:15Z

/retest-required

jcmoraisjr · 2026-05-25T18:20:58Z

/lgtm

Looks great!

rikatz · 2026-05-26T13:43:06Z

+		UseDualStack: aws.Bool(false),
+	})
+	if err != nil {
+		return ""


silently ignoring without logging, we need to log

rikatz · 2026-05-26T13:54:24Z

@coderabbitai I want you to do a new full adversarial review, line by line of the changes and the proposal, but keep yourself in the current context of the change: bumping to aws-sdk-go-v2. Is there any good reason not to merge this? Also ignore any changes on the vendor/ directory, and focus on the main code

coderabbitai · 2026-05-26T13:56:23Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== pkg/dns/aws/dns.go ==="
cat pkg/dns/aws/dns.go

echo ""
echo "=== pkg/dns/aws/dns_test.go ==="
cat pkg/dns/aws/dns_test.go

echo ""
echo "=== pkg/util/aws/shared_credentials_file.go ==="
cat pkg/util/aws/shared_credentials_file.go

echo ""
echo "=== pkg/util/aws/shared_credentials_file_test.go ==="
cat pkg/util/aws/shared_credentials_file_test.go

echo ""
echo "=== test/e2e/aws_util_test.go ==="
cat test/e2e/aws_util_test.go

echo ""
echo "=== test/e2e/lb_eip_test.go ==="
cat test/e2e/lb_eip_test.go

echo ""
echo "=== test/e2e/operator_test.go — imports + assertEIPAllocationDeleted + ioutil usage ==="
head -80 test/e2e/operator_test.go
grep -n "ioutil\|io\.Discard\|io\.ReadAll\|assertEIPAlloc\|createEC2\|getPublicSubnet\|getVPCId" test/e2e/operator_test.go | head -50

echo ""
echo "=== go.mod (AWS / smithy lines) ==="
grep -n "aws\|smithy\|jmespath\|signin" go.mod

echo ""
echo "=== Check GovCloud region helpers ==="
grep -n -A 10 "func isGovCloudRegion\|func partitionIDForRegion" pkg/dns/aws/dns.go

echo ""
echo "=== Check all smithy / error handling in dns.go ==="
grep -n -B2 -A8 "smithy\|errors\.As\|not found\|InvalidChangeBatch\|ErrorCode\|ErrorMessage" pkg/dns/aws/dns.go

echo ""
echo "=== Check context usage in dns.go ==="
grep -n "context\." pkg/dns/aws/dns.go | head -40

echo ""
echo "=== Check all pagination calls ==="
grep -n "NewGetResourcesPaginator\|NewListHostedZonesPaginator\|NewDescribeLoadBalancersPaginator\|HasMorePages\|NextPage" pkg/dns/aws/dns.go

echo ""
echo "=== Check validateServiceEndpoints ==="
grep -n -A 60 "func.*validateServiceEndpoints" pkg/dns/aws/dns.go

echo ""
echo "=== Check resolveEndpoint ==="
grep -n -A 80 "func.*resolveEndpoint" pkg/dns/aws/dns.go

echo ""
echo "=== Check RoleARN assumption code ==="
grep -n -A 20 "RoleARN" pkg/dns/aws/dns.go

echo ""
echo "=== Check updateRecord full function ==="
grep -n -A 120 "func.*updateRecord" pkg/dns/aws/dns.go

echo ""
echo "=== Check Delete function ==="
grep -n -A 30 "func.*Delete\b" pkg/dns/aws/dns.go

echo ""
echo "=== Check lookupZoneID variants ==="
grep -n -A 60 "func.*lookupZoneID" pkg/dns/aws/dns.go

echo ""
echo "=== Check all constants ==="
grep -n "const\|= \"" pkg/dns/aws/dns.go | head -60

echo ""
echo "=== Check for any TODO/FIXME/HACK in changed non-vendor files ==="
grep -rn "TODO\|FIXME\|HACK\|XXX" pkg/dns/aws/ pkg/util/aws/ test/e2e/aws_util_test.go test/e2e/lb_eip_test.go 2>/dev/null | grep -v "vendor" | head -30

echo ""
echo "=== Check for nil pointer risks in dns.go ==="
grep -n "\.Tags\b\|\.ResourceARN\b\|\.HostedZoneId\b\|aws\.ToString" pkg/dns/aws/dns.go | head -40

echo ""
echo "=== Check test coverage for GovCloud specific paths ==="
grep -n "gov\|GovCloud\|us-gov" pkg/dns/aws/dns_test.go | head -30

echo ""
echo "=== Check v1 pre-migration commit for NewProvider to see credential file handling ==="
git show d16dd454:pkg/dns/aws/dns.go 2>/dev/null | grep -n -B5 -A15 "SharedCredentialFile\|NewSessionWithOptions\|session\.Must"

Length of output: 105849

@rikatz — here is the final adversarial line-by-line pass on all non-vendor changed files.

Final Adversarial Review

✅ Everything Previously Flagged — Confirmed Resolved or Invalid

All findings from the prior two rounds have been addressed, confirmed as v1 parity behavior, or confirmed as inherent v2 limitations with no regression impact. No re-hash needed.

🔍 Fresh Pass — New Items Examined

1. GovCloud us-gov-east-1 tagging override (Line 358) — improved over v1

if region == "us-gov-east-1" {
    log.Info("Ignoring custom resourcegroupstaggingapi endpoint...")
} else {
    tagEndpoint = ep.URL
}

This is actually better than v1, which used strings.Contains(ep.URL, "us-gov-east-1") (URL substring). The v2 code compares against the actual region string. tagFound is still set to true when the endpoint is ignored, which correctly prevents any further tagging endpoint entries from being processed. Test case "custom service endpoints, GovCloud East ignores custom tagging" explicitly covers this path. ✅

2. lookupZoneIDWithoutResourceTagging — empty-page guard (Line 562)

if len(resp.HostedZones) == 0 {
    continue
}

This guard was added in the v2 migration and is actually more defensive than v1. It prevents calling ListTagsForResources with an empty resourceIds slice on a paginator pass that yields no zones. ✅

3. resolveEndpoint functions now use the SDK's own endpoint resolver (Lines 168–244)

In v1, endpoint strings were hand-constructed (fmt.Sprintf(…)). In v2, each resolveXEndpoint function calls the SDK's own NewDefaultEndpointResolverV2(). This means the logged/tested endpoints are derived from the same resolver the SDK itself uses — significantly stronger than the v1 approach. ✅

4. Lock discipline — no new races (Lines 482–670)

getZoneID and getLBHostedZone each acquire m.lock independently. The change function calls them in sequence without holding the lock itself (correct — neither call needs atomicity with the other). Matches v1 discipline exactly. ✅

5. TaggingService = "tagging" vs SDK service ID (Lines 44–46)

The comment above the constants block explicitly documents why these are hardcoded (aws-sdk-go-v2 exports human-readable ServiceID values like "Route 53", not the endpoint prefix strings that AWSServiceEndpoint.Name uses). The hardcoded values are correct for their purpose and match what OpenShift's infrastructure API expects. ✅

6. service/signin v1.0.11 indirect dep

No Go file in the repo (or vendor) directly imports this package. It's a transitive dependency of service/sso/service/ssooidc used for IAM Identity Center credential chaining — a standard part of the v2 SDK dependency graph. ✅

7. Test coverage completeness

Test_NewProvider covers: us-east-1 (standard), eusc-de-east-1 (default + custom), us-gov-east-1 (default + custom with tagging ignore), us-gov-west-1 (default + custom with tagging accepted), us-iso-east-1 (C2S, no tagging), us-isob-east-1 (SC2S, no tagging), cn-north-1 (China). Test_partitionIDForRegion and Test_isGovCloudRegion cover all partition prefix cases. ✅

Remaining Items (not blocking, all pre-existing v1 behavior)


`context.TODO()` throughout	Pre-existing v1 behavior; context propagation is a future improvement, out of scope for a migration PR
`strings.Contains(apiErr.ErrorMessage(), "not found")` in delete path	Pre-existing v1 behavior; adding `apiErr.ErrorCode() == "InvalidChangeBatch"` would be a safe improvement but is not a regression

Verdict

There is no code-level reason not to merge this PR. The migration is:

Behaviorally faithful to v1 across all partitions (standard, GovCloud, C2S, SC2S, China, EUSC)
Well-tested with unit coverage for all region/partition branches
Passing CI including the e2e-aws-operator suite (including the delete-path test that failed on an earlier commit)
Meaningfully improved in some areas (endpoint resolution uses SDK resolver, GovCloud tagging override uses region comparison instead of URL substring)

The /hold you placed for GovCloud live testing is the right call as an operational gate — that's not a code quality concern, and the unit tests for GovCloud East/West already verify the expected endpoint URLs (https://route53.us-gov.amazonaws.com, https://tagging.us-gov-west-1.amazonaws.com). Once you get confirmation from GovCloud testing, this is ready to go.

coderabbitai

🧹 Nitpick comments (9)

pkg/dns/aws/dns.go (9)

558-560: ⚡ Quick win

Use %w for error wrapping.

Proposed fix

 		resp, err := paginator.NextPage(ctx)
 		if err != nil {
-			return "", fmt.Errorf("failed to list hosted zones: %v", err)
+			return "", fmt.Errorf("failed to list hosted zones: %w", err)
 		}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/dns/aws/dns.go` around lines 558 - 560, The error returned from
paginator.NextPage(ctx) is being wrapped with fmt.Errorf using %v; change the
wrapping to use %w so the original error is preserved for unwrapping. Locate the
block where paginator.NextPage(ctx) is called (the paginator.NextPage call and
the fmt.Errorf on failure) and replace the fmt.Errorf("failed to list hosted
zones: %v", err) usage with fmt.Errorf("failed to list hosted zones: %w", err)
to enable proper error wrapping/unwrapping.

757-759: ⚡ Quick win

Use %w for error wrapping.

Proposed fix

 	err = m.updateRecord(domain, zoneID, target, targetHostedZoneID, string(action), record.Spec.RecordTTL)
 	if err != nil {
-		return fmt.Errorf("failed to update alias in zone %s: %v", zoneID, err)
+		return fmt.Errorf("failed to update alias in zone %s: %w", zoneID, err)
 	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/dns/aws/dns.go` around lines 757 - 759, The fmt.Errorf call that reports
an error from m.updateRecord should wrap the original error using %w instead of
%v so callers can inspect/unwrap it; locate the call to m.updateRecord (passing
domain, zoneID, target, targetHostedZoneID, string(action),
record.Spec.RecordTTL) and change the fmt.Errorf("failed to update alias in zone
%s: %v", zoneID, err) to use %w (e.g., fmt.Errorf("failed to update alias in
zone %s: %w", zoneID, err)) so the error is properly wrapped.

269-269: ⚡ Quick win

Use %w for error wrapping to enable errors.Is()/errors.As() unwrapping.

Per coding guidelines, wrapped errors should use %w instead of %v.

Proposed fix

-		return nil, fmt.Errorf("couldn't create AWS client config: %v", err)
+		return nil, fmt.Errorf("couldn't create AWS client config: %w", err)

As per coding guidelines: "Return errors with context using fmt.Errorf("<action>: %w", err) and always use %w for wrapped error values to allow errors.Is()/errors.As() unwrapping"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/dns/aws/dns.go` at line 269, Replace the fmt.Errorf call that currently
uses "%v" to wrap the underlying error with "%w" so callers can use
errors.Is()/errors.As(); specifically change the return that reads
`fmt.Errorf("couldn't create AWS client config: %v", err)` to use
`fmt.Errorf("couldn't create AWS client config: %w", err)` (locate this in
pkg/dns/aws/dns.go in the AWS client config creation/initialization block).

453-472: ⚡ Quick win

Use %w for error wrapping in validation errors.

Multiple error wrappings in this function use %v instead of %w.

Proposed fix

 	if _, err := provider.route53.ListHostedZones(ctx, &zoneInput); err != nil {
-		errs = append(errs, fmt.Errorf("failed to list route53 hosted zones: %v", err))
+		errs = append(errs, fmt.Errorf("failed to list route53 hosted zones: %w", err))
 	}
 	elbInput := elasticloadbalancing.DescribeLoadBalancersInput{PageSize: aws.Int32(1)}
 	if _, err := provider.elb.DescribeLoadBalancers(ctx, &elbInput); err != nil {
-		errs = append(errs, fmt.Errorf("failed to describe elb load balancers: %v", err))
+		errs = append(errs, fmt.Errorf("failed to describe elb load balancers: %w", err))
 	}
 	elbv2Input := elbv2.DescribeLoadBalancersInput{PageSize: aws.Int32(1)}
 	if _, err := provider.elbv2.DescribeLoadBalancers(ctx, &elbv2Input); err != nil {
-		errs = append(errs, fmt.Errorf("failed to describe elbv2 load balancers: %v", err))
+		errs = append(errs, fmt.Errorf("failed to describe elbv2 load balancers: %w", err))
 	}
 	if provider.tags != nil {
 		tagInput := resourcegroupstaggingapi.GetResourcesInput{TagsPerPage: aws.Int32(100)}
 		if _, err := provider.tags.GetResources(ctx, &tagInput); err != nil {
-			errs = append(errs, fmt.Errorf("failed to get group tagging resources: %v", err))
+			errs = append(errs, fmt.Errorf("failed to get group tagging resources: %w", err))
 		}
 	}

As per coding guidelines: "Return errors with context using fmt.Errorf("<action>: %w", err) and always use %w for wrapped error values"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/dns/aws/dns.go` around lines 453 - 472, The error constructions in this
validation block (the errs slice population around
provider.route53.ListHostedZones, provider.elb.DescribeLoadBalancers,
provider.elbv2.DescribeLoadBalancers, and provider.tags.GetResources) use
fmt.Errorf with "%v"; replace each fmt.Errorf("...: %v", err) with
fmt.Errorf("...: %w", err) so errors are properly wrapped (e.g., change
fmt.Errorf("failed to list route53 hosted zones: %v", err) to fmt.Errorf("failed
to list route53 hosted zones: %w", err)); keep the same messages and return
kerrors.NewAggregate(errs) as before.

825-836: ⚡ Quick win

Use %w for error wrapping.

Proposed fix

 	resp, err := m.route53.ChangeResourceRecordSets(context.TODO(), &input)
 	if err != nil {
 		if action == string(deleteAction) {
 			var apiErr smithy.APIError
 			if errors.As(err, &apiErr) {
 				if strings.Contains(apiErr.ErrorMessage(), "not found") {
 					log.Info("record not found", "zone id", zoneID, "domain", domain, "target", target)
 					return nil
 				}
 			}
 		}
-		return fmt.Errorf("couldn't update DNS record in zone %s: %v", zoneID, err)
+		return fmt.Errorf("couldn't update DNS record in zone %s: %w", zoneID, err)
 	}

As per coding guidelines: "Return errors with context using fmt.Errorf("<action>: %w", err)"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/dns/aws/dns.go` around lines 825 - 836, The error return uses fmt.Errorf
with %v which loses the wrapped error; change the return to wrap the original
error using %w so callers can inspect it. Specifically, in the block after
calling ChangeResourceRecordSets (around ChangeResourceRecordSets, deleteAction
and apiErr), replace fmt.Errorf("couldn't update DNS record in zone %s: %v",
zoneID, err) with fmt.Errorf("couldn't update DNS record in zone %s: %w",
zoneID, err) so the original error is preserved and can be unwrapped.

536-543: ⚡ Quick win

Use %w for error wrapping in lookupZoneID.

Proposed fix

 		resp, err := paginator.NextPage(ctx)
 		if err != nil {
-			return "", fmt.Errorf("failed to get tagged resources: %v", err)
+			return "", fmt.Errorf("failed to get tagged resources: %w", err)
 		}
 		for _, zone := range resp.ResourceTagMappingList {
 			zoneARN, err := arn.Parse(aws.ToString(zone.ResourceARN))
 			if err != nil {
-				return "", fmt.Errorf("failed to parse hostedzone ARN %q: %v", aws.ToString(zone.ResourceARN), err)
+				return "", fmt.Errorf("failed to parse hostedzone ARN %q: %w", aws.ToString(zone.ResourceARN), err)
 			}

As per coding guidelines: "Return errors with context using fmt.Errorf("<action>: %w", err)"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/dns/aws/dns.go` around lines 536 - 543, In lookupZoneID, the error
returns use %v instead of wrapping the underlying error; change the fmt.Errorf
calls that follow paginator.NextPage(ctx) and arn.Parse(...) to use %w so
callers can inspect the original error (e.g. replace fmt.Errorf("failed to get
tagged resources: %v", err) with fmt.Errorf("failed to get tagged resources:
%w", err) and fmt.Errorf("failed to parse hostedzone ARN %q: %v",
aws.ToString(zone.ResourceARN), err) with fmt.Errorf("failed to parse hostedzone
ARN %q: %w", aws.ToString(zone.ResourceARN), err)); keep the same messages and
context and ensure this applies to any other fmt.Errorf in lookupZoneID that
wraps returned errors.

632-654: ⚡ Quick win

Use %w for error wrapping in ELB discovery.

Proposed fix

 		resp, err := elbPaginator.NextPage(ctx)
 		if err != nil {
-			return "", fmt.Errorf("failed to describe classic load balancers: %v", err)
+			return "", fmt.Errorf("failed to describe classic load balancers: %w", err)
 		}

 			resp, err := elbv2Paginator.NextPage(ctx)
 			if err != nil {
-				return "", fmt.Errorf("failed to describe network load balancers: %v", err)
+				return "", fmt.Errorf("failed to describe network load balancers: %w", err)
 			}

As per coding guidelines: "Return errors with context using fmt.Errorf("<action>: %w", err)"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/dns/aws/dns.go` around lines 632 - 654, The error returns from
elbPaginator.NextPage and elbv2Paginator.NextPage currently use fmt.Errorf with
%v (e.g., fmt.Errorf("failed to describe classic load balancers: %v", err)), so
update both to wrap the original error using %w (e.g., fmt.Errorf("failed to
describe classic load balancers: %w", err)) for elbPaginator.NextPage and
similarly for elbv2Paginator.NextPage to ensure proper error wrapping; change
both fmt.Errorf calls that reference err to use %w.

444-444: ⚡ Quick win

Use %w for error wrapping.

Proposed fix

-		return nil, fmt.Errorf("failed to validate aws provider service endpoints: %v", err)
+		return nil, fmt.Errorf("failed to validate aws provider service endpoints: %w", err)

As per coding guidelines: "Return errors with context using fmt.Errorf("<action>: %w", err) and always use %w for wrapped error values"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/dns/aws/dns.go` at line 444, Replace the fmt.Errorf call that returns the
validation error in pkg/dns/aws (the return nil, fmt.Errorf("failed to validate
aws provider service endpoints: %v", err) statement) to wrap the original error
with %w instead of %v; update the error formatting so the returned error uses
fmt.Errorf("failed to validate aws provider service endpoints: %w", err) to
preserve error wrapping semantics.

712-720: ⚡ Quick win

Use %w for error wrapping.

Proposed fix

 	zoneID, err := m.getZoneID(zone)
 	if err != nil {
-		return fmt.Errorf("failed to find hosted zone for record: %v", err)
+		return fmt.Errorf("failed to find hosted zone for record: %w", err)
 	}

 	// Find the target hosted zone of the load balancer attached to the service.
 	targetHostedZoneID, err := m.getLBHostedZone(target)
 	if err != nil {
-		err = fmt.Errorf("failed to get hosted zone for load balancer target %q: %v", target, err)
+		err = fmt.Errorf("failed to get hosted zone for load balancer target %q: %w", target, err)

As per coding guidelines: "Return errors with context using fmt.Errorf("<action>: %w", err)"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/dns/aws/dns.go` around lines 712 - 720, The error constructed when
getLBHostedZone(target) fails uses fmt.Errorf with %v instead of wrapping the
original error; update the error assignment in the block that calls
getLBHostedZone (where targetHostedZoneID, err := m.getLBHostedZone(target) is
handled) to wrap the original err using fmt.Errorf("failed to get hosted zone
for load balancer target %q: %w", target, err) so the underlying error is
preserved and can be unwrapped.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@pkg/dns/aws/dns.go`:
- Around line 558-560: The error returned from paginator.NextPage(ctx) is being
wrapped with fmt.Errorf using %v; change the wrapping to use %w so the original
error is preserved for unwrapping. Locate the block where
paginator.NextPage(ctx) is called (the paginator.NextPage call and the
fmt.Errorf on failure) and replace the fmt.Errorf("failed to list hosted zones:
%v", err) usage with fmt.Errorf("failed to list hosted zones: %w", err) to
enable proper error wrapping/unwrapping.
- Around line 757-759: The fmt.Errorf call that reports an error from
m.updateRecord should wrap the original error using %w instead of %v so callers
can inspect/unwrap it; locate the call to m.updateRecord (passing domain,
zoneID, target, targetHostedZoneID, string(action), record.Spec.RecordTTL) and
change the fmt.Errorf("failed to update alias in zone %s: %v", zoneID, err) to
use %w (e.g., fmt.Errorf("failed to update alias in zone %s: %w", zoneID, err))
so the error is properly wrapped.
- Line 269: Replace the fmt.Errorf call that currently uses "%v" to wrap the
underlying error with "%w" so callers can use errors.Is()/errors.As();
specifically change the return that reads `fmt.Errorf("couldn't create AWS
client config: %v", err)` to use `fmt.Errorf("couldn't create AWS client config:
%w", err)` (locate this in pkg/dns/aws/dns.go in the AWS client config
creation/initialization block).
- Around line 453-472: The error constructions in this validation block (the
errs slice population around provider.route53.ListHostedZones,
provider.elb.DescribeLoadBalancers, provider.elbv2.DescribeLoadBalancers, and
provider.tags.GetResources) use fmt.Errorf with "%v"; replace each
fmt.Errorf("...: %v", err) with fmt.Errorf("...: %w", err) so errors are
properly wrapped (e.g., change fmt.Errorf("failed to list route53 hosted zones:
%v", err) to fmt.Errorf("failed to list route53 hosted zones: %w", err)); keep
the same messages and return kerrors.NewAggregate(errs) as before.
- Around line 825-836: The error return uses fmt.Errorf with %v which loses the
wrapped error; change the return to wrap the original error using %w so callers
can inspect it. Specifically, in the block after calling
ChangeResourceRecordSets (around ChangeResourceRecordSets, deleteAction and
apiErr), replace fmt.Errorf("couldn't update DNS record in zone %s: %v", zoneID,
err) with fmt.Errorf("couldn't update DNS record in zone %s: %w", zoneID, err)
so the original error is preserved and can be unwrapped.
- Around line 536-543: In lookupZoneID, the error returns use %v instead of
wrapping the underlying error; change the fmt.Errorf calls that follow
paginator.NextPage(ctx) and arn.Parse(...) to use %w so callers can inspect the
original error (e.g. replace fmt.Errorf("failed to get tagged resources: %v",
err) with fmt.Errorf("failed to get tagged resources: %w", err) and
fmt.Errorf("failed to parse hostedzone ARN %q: %v",
aws.ToString(zone.ResourceARN), err) with fmt.Errorf("failed to parse hostedzone
ARN %q: %w", aws.ToString(zone.ResourceARN), err)); keep the same messages and
context and ensure this applies to any other fmt.Errorf in lookupZoneID that
wraps returned errors.
- Around line 632-654: The error returns from elbPaginator.NextPage and
elbv2Paginator.NextPage currently use fmt.Errorf with %v (e.g.,
fmt.Errorf("failed to describe classic load balancers: %v", err)), so update
both to wrap the original error using %w (e.g., fmt.Errorf("failed to describe
classic load balancers: %w", err)) for elbPaginator.NextPage and similarly for
elbv2Paginator.NextPage to ensure proper error wrapping; change both fmt.Errorf
calls that reference err to use %w.
- Line 444: Replace the fmt.Errorf call that returns the validation error in
pkg/dns/aws (the return nil, fmt.Errorf("failed to validate aws provider service
endpoints: %v", err) statement) to wrap the original error with %w instead of
%v; update the error formatting so the returned error uses fmt.Errorf("failed to
validate aws provider service endpoints: %w", err) to preserve error wrapping
semantics.
- Around line 712-720: The error constructed when getLBHostedZone(target) fails
uses fmt.Errorf with %v instead of wrapping the original error; update the error
assignment in the block that calls getLBHostedZone (where targetHostedZoneID,
err := m.getLBHostedZone(target) is handled) to wrap the original err using
fmt.Errorf("failed to get hosted zone for load balancer target %q: %w", target,
err) so the underlying error is preserved and can be unwrapped.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 7d82550a-1cb6-40b9-b2ab-62bcaf2bbed8

📥 Commits

Reviewing files that changed from the base of the PR and between b7a92db and 7366a45.

📒 Files selected for processing (1)

pkg/dns/aws/dns.go

rikatz · 2026-05-26T14:03:09Z

/hold cancel

this now seems ready to go 👋

jcmoraisjr · 2026-05-26T14:12:36Z

/lgtm

rikatz · 2026-05-26T14:41:41Z

/cc @rhamini3 @melvinjoseph86

melvinjoseph86 · 2026-06-02T11:20:09Z

/test list

openshift-ci · 2026-06-02T11:20:29Z

@melvinjoseph86: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test e2e-aws-operator

/test e2e-aws-ovn

/test e2e-aws-ovn-hypershift-conformance

/test e2e-aws-ovn-serial-1of2

/test e2e-aws-ovn-serial-2of2

/test e2e-aws-ovn-upgrade

/test e2e-azure-operator

/test e2e-gcp-operator

/test e2e-hypershift

/test hypershift-e2e-aks

/test images

/test okd-scos-images

/test unit

/test verify

/test verify-deps

The following commands are available to trigger optional jobs:

/test e2e-aws-gatewayapi-conformance

/test e2e-aws-gatewayapi-conformance-techpreview

/test e2e-aws-operator-techpreview

/test e2e-aws-ovn-single-node

/test e2e-aws-ovn-techpreview

/test e2e-aws-pre-release-ossm

/test e2e-azure-manual-oidc

/test e2e-azure-ovn

/test e2e-baremetalds-metallb-gatewayapi-conformance

/test e2e-baremetalds-metallb-gatewayapi-conformance-techpreview

/test e2e-gcp-ovn

/test e2e-ibmcloud-operator

/test e2e-openstack-operator

/test e2e-vsphere-static-metallb-gatewayapi-conformance

/test e2e-vsphere-static-metallb-gatewayapi-conformance-techpreview

/test e2e-vsphere-static-metallb-operator-gwapi

/test e2e-vsphere-static-metallb-operator-gwapi-techpreview

/test okd-scos-e2e-aws-ovn

Use /test all to run the following jobs that were automatically triggered:

pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-operator

pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-operator-techpreview

pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-ovn

pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-ovn-hypershift-conformance

pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-ovn-serial-1of2

pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-ovn-serial-2of2

pull-ci-openshift-cluster-ingress-operator-master-e2e-aws-ovn-upgrade

pull-ci-openshift-cluster-ingress-operator-master-e2e-azure-operator

pull-ci-openshift-cluster-ingress-operator-master-e2e-gcp-operator

pull-ci-openshift-cluster-ingress-operator-master-e2e-hypershift

pull-ci-openshift-cluster-ingress-operator-master-e2e-vsphere-static-metallb-operator-gwapi

pull-ci-openshift-cluster-ingress-operator-master-e2e-vsphere-static-metallb-operator-gwapi-techpreview

pull-ci-openshift-cluster-ingress-operator-master-hypershift-e2e-aks

pull-ci-openshift-cluster-ingress-operator-master-images

pull-ci-openshift-cluster-ingress-operator-master-okd-scos-images

pull-ci-openshift-cluster-ingress-operator-master-unit

pull-ci-openshift-cluster-ingress-operator-master-verify

pull-ci-openshift-cluster-ingress-operator-master-verify-deps

Details

In response to this:

/test list

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

melvinjoseph86 · 2026-06-02T11:56:44Z

/payload-job periodic-ci-openshift-openshift-tests-private-release-5.0-amd64-nightly-aws-usgov-ipi-custom-dns-mini-perm-tp-f7

openshift-ci · 2026-06-02T11:56:47Z

@melvinjoseph86: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-openshift-tests-private-release-5.0-amd64-nightly-aws-usgov-ipi-custom-dns-mini-perm-tp-f7

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/20c330c0-5e7a-11f1-9fe2-13e38a39695c-0

melvinjoseph86 · 2026-06-02T11:57:56Z

/payload-job periodic-ci-openshift-openshift-tests-private-release-5.0-amd64-nightly-aws-usgov-ipi-private-ep-fips-f7 periodic-ci-openshift-openshift-tests-private-release-5.0-amd64-nightly-aws-usgov-ipi-private-sts-f7

openshift-ci · 2026-06-02T11:58:02Z

@melvinjoseph86: trigger 2 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-openshift-tests-private-release-5.0-amd64-nightly-aws-usgov-ipi-private-ep-fips-f7
periodic-ci-openshift-openshift-tests-private-release-5.0-amd64-nightly-aws-usgov-ipi-private-sts-f7

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/4bf89780-5e7a-11f1-8850-f848f373f480-0

melvinjoseph86 · 2026-06-03T09:17:17Z

/payload-job periodic-ci-openshift-openshift-tests-private-release-5.0-amd64-nightly-aws-usgov-ipi-private-ep-fips-f7 periodic-ci-openshift-openshift-tests-private-release-5.0-amd64-nightly-aws-usgov-ipi-private-sts-f7 periodic-ci-openshift-openshift-tests-private-release-5.0-amd64-nightly-aws-usgov-ipi-custom-dns-mini-perm-tp-f7

openshift-ci · 2026-06-03T09:17:21Z

@melvinjoseph86: trigger 3 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-openshift-tests-private-release-5.0-amd64-nightly-aws-usgov-ipi-private-ep-fips-f7
periodic-ci-openshift-openshift-tests-private-release-5.0-amd64-nightly-aws-usgov-ipi-private-sts-f7
periodic-ci-openshift-openshift-tests-private-release-5.0-amd64-nightly-aws-usgov-ipi-custom-dns-mini-perm-tp-f7

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/04514cc0-5f2d-11f1-87b2-b2ae608b681d-0

melvinjoseph86 · 2026-06-03T18:44:31Z

/payload-job periodic-ci-openshift-openshift-tests-private-release-5.0-amd64-nightly-aws-usgov-ipi-private-ep-fips-f7 periodic-ci-openshift-openshift-tests-private-release-5.0-amd64-nightly-aws-usgov-ipi-private-sts-f7 periodic-ci-openshift-openshift-tests-private-release-5.0-amd64-nightly-aws-usgov-ipi-custom-dns-mini-perm-tp-f7

openshift-ci · 2026-06-03T18:44:42Z

@melvinjoseph86: trigger 3 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-openshift-tests-private-release-5.0-amd64-nightly-aws-usgov-ipi-private-ep-fips-f7
periodic-ci-openshift-openshift-tests-private-release-5.0-amd64-nightly-aws-usgov-ipi-private-sts-f7
periodic-ci-openshift-openshift-tests-private-release-5.0-amd64-nightly-aws-usgov-ipi-custom-dns-mini-perm-tp-f7

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/428e4700-5f7c-11f1-8cad-766508e7bd57-0

melvinjoseph86 · 2026-06-04T14:13:32Z

Tested with cluster bot and automation run with OCP48138 passes

passed: (3m54s) 2026-06-03T18:38:24 "[sig-network-edge] Network_Edge Component_ExtDNS Author:hongli-ConnectedOnly-ROSA-OSD_CCS-LEVEL0-High-48138-Support External DNS on AWS platform"

Writing JUnit report to junit_e2e_20260603-183824.xml

1 pass, 0 skip (3m54s)

Also able to create the dnsrecord correctly in internal LB:

abc123.gwapi.apps.mjoseph-awsg.qe.devcluster.openshift.com. 30 IN CNAME	internal-a365d5626f7ff47058436b3b9df7d3ad-459766124.us-gov-east-1.elb.amazonaws.com.
internal-a365d5626f7ff47058436b3b9df7d3ad-459766124.us-gov-east-1.elb.amazonaws.com. 30	IN A 10.0.67.112
internal-a365d5626f7ff47058436b3b9df7d3ad-459766124.us-gov-east-1.elb.amazonaws.com. 30	IN A 10.0.61.215

Hence marking as verified
/verified by @mjoseph

openshift-ci-robot · 2026-06-04T14:13:45Z

@melvinjoseph86: This PR has been marked as verified by @mjoseph.

Details

In response to this:

Tested with cluster bot and automation run with OCP48138 passes

passed: (3m54s) 2026-06-03T18:38:24 "[sig-network-edge] Network_Edge Component_ExtDNS Author:hongli-ConnectedOnly-ROSA-OSD_CCS-LEVEL0-High-48138-Support External DNS on AWS platform"

Writing JUnit report to junit_e2e_20260603-183824.xml

1 pass, 0 skip (3m54s)

Also able to create the dnsrecord correctly in internal LB:

abc123.gwapi.apps.mjoseph-awsg.qe.devcluster.openshift.com. 30 IN CNAME	internal-a365d5626f7ff47058436b3b9df7d3ad-459766124.us-gov-east-1.elb.amazonaws.com.
internal-a365d5626f7ff47058436b3b9df7d3ad-459766124.us-gov-east-1.elb.amazonaws.com. 30	IN A 10.0.67.112
internal-a365d5626f7ff47058436b3b9df7d3ad-459766124.us-gov-east-1.elb.amazonaws.com. 30	IN A 10.0.61.215

Hence marking as verified
/verified by @mjoseph

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci · 2026-06-04T18:01:01Z

@rikatz: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label May 13, 2026

openshift-ci Bot requested review from Thealisyed and knobunc May 13, 2026 21:37

coderabbitai Bot reviewed May 13, 2026

View reviewed changes

rikatz added 2 commits May 13, 2026 19:03

NE-2723: Migrate to aws-sdk-go-v2

4194007

The aws-sdk-go library, used on DNS controller is deprecated and does not receive updates anymore. This change migrates to the new aws-sdk-go-v2, adding the new library and making the required changes to keep the code compatible

NE-2723: vendor new aws-sdk-go-v2 libraries

6d8c121

rikatz force-pushed the upgrade-aws-sdk branch from 9470c72 to 6d8c121 Compare May 13, 2026 22:03

NE-2723: Address coderabbit comments

0d72f79

openshift-ci Bot assigned jcmoraisjr May 20, 2026

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label May 25, 2026

openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 26, 2026

fix endpoint resolver to use sdk resolver

b7a92db

openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label May 26, 2026

rikatz commented May 26, 2026

View reviewed changes

Add missing logging to avoid silent drop

7366a45

coderabbitai Bot reviewed May 26, 2026

View reviewed changes

openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 26, 2026

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label May 26, 2026

openshift-ci Bot requested review from melvinjoseph86 and rhamini3 May 26, 2026 14:41

openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Jun 4, 2026

openshift-merge-bot Bot merged commit b54ad93 into openshift:master Jun 4, 2026
20 checks passed

Uh oh!

Conversation

rikatz commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

Changes

Uh oh!

openshift-ci-robot commented May 13, 2026 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Pre-merge checks failed

❌ Failed checks (1 error, 1 warning, 1 inconclusive)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

rikatz commented May 13, 2026

Uh oh!

coderabbitai Bot commented May 13, 2026

🔴 Critical / Breaking Issues

1. GovCloud detection change: C2S/SC2S now use Alias records instead of CNAME

2. lookupZoneID: Returns on first match without validating uniqueness

3. SharedCredentialFile as both credentials and config file

🟠 Major Issues

4. resolveEndpoint is logging-only but tests assert against it

5. GovCloud tagging endpoint override is fragile string matching

6. break inside switch default case for EUSC — potentially confusing

7. Test_partitionIDForRegion: eusc-de-east-1 maps to "aws" — no test for EUSC-specific behavior in NewProvider with default endpoints

8. Dual-stack not handled for GovCloud CNAME records

🟡 Minor Issues

9. rand.Seed is deprecated (Go ≥ 1.20)

10. io/ioutil import not cleaned up in operator_test.go

11. context.TODO() used pervasively instead of propagated contexts

12. No validation that resolveEndpoint returns a non-empty string for all supported partitions

13. cfgRoute53 := cfg shallow copy — credential sharing risk

Summary Table

Uh oh!

rikatz commented May 13, 2026

Uh oh!

coderabbitai Bot commented May 13, 2026

Uh oh!

rikatz commented May 13, 2026

Uh oh!

coderabbitai Bot commented May 13, 2026

Acknowledged Invalid Findings (with v1 evidence)

Fresh Line-by-Line Re-Review

🟠 Remaining Major Issue

🟡 Remaining Minor Issues

Summary of Remaining Items

Uh oh!

rikatz commented May 14, 2026

Uh oh!

coderabbitai Bot commented May 14, 2026

Fresh Adversarial Re-Review

✅ Correction from previous round: Finding #7 is also invalid

🔴 Reason Not to Merge #1 — CI failure is DNS-specific and likely PR-related

🟠 Reason Not to Merge #2 — updateRecord delete error detection uses string matching instead of error code

🟡 Minor Issues (not blockers, but adversarial completeness)

3. lookupZoneIDWithoutResourceTagging — ListTagsForResources with empty slice

4. service/signin v1.0.11 in go.mod is unusual

5. govCloudRoute53Region constant dual use is easy to misread

CI Failure Verdict

Uh oh!

rikatz commented May 14, 2026

Uh oh!

rikatz commented May 15, 2026

Uh oh!

rikatz commented May 15, 2026

Uh oh!

coderabbitai Bot commented May 15, 2026

Final Adversarial Line-by-Line Review

🟢 Items Previously Flagged — Confirmed Resolved or Invalid

🔍 New Pass — Items Examined Fresh

Verdict

Uh oh!

rikatz commented May 15, 2026

rikatz commented May 13, 2026 •

edited

Loading

openshift-ci-robot commented May 13, 2026 •

edited by openshift-ci Bot

Loading

coderabbitai Bot commented May 13, 2026 •

edited

Loading

2. `lookupZoneID`: Returns on first match without validating uniqueness

3. `SharedCredentialFile` as both credentials and config file

4. `resolveEndpoint` is logging-only but tests assert against it

6. `break` inside `switch default` case for EUSC — potentially confusing

7. `Test_partitionIDForRegion`: `eusc-de-east-1` maps to `"aws"` — no test for EUSC-specific behavior in `NewProvider` with default endpoints

9. `rand.Seed` is deprecated (Go ≥ 1.20)

10. `io/ioutil` import not cleaned up in `operator_test.go`

11. `context.TODO()` used pervasively instead of propagated contexts

12. No validation that `resolveEndpoint` returns a non-empty string for all supported partitions

13. `cfgRoute53 := cfg` shallow copy — credential sharing risk

✅ Correction from previous round: Finding `#7` is also invalid

🔴 Reason Not to Merge `#1` — CI failure is DNS-specific and likely PR-related

🟠 Reason Not to Merge `#2` — `updateRecord` delete error detection uses string matching instead of error code

3. `lookupZoneIDWithoutResourceTagging` — `ListTagsForResources` with empty slice

4. `service/signin v1.0.11` in `go.mod` is unusual

5. `govCloudRoute53Region` constant dual use is easy to misread

`ci/prow/e2e-aws-ovn` — CI Infrastructure Failure ❌ (not PR-related)

`ci/prow/e2e-aws-ovn-serial-1of2` — Unrelated sig-cli Test Failure ❌ (not PR-related)

`ci/prow/e2e-aws-ovn-serial-2of2` — Platform MonitorTest Failure ❌ (not PR-related)